1605244 - C-C TB - Do not call read when there is no characters available.

Status: RESOLVED FIXED

(Thunderbird :: Filters, task)

Description

[ISHIKAWA, Chiaki]

Attachment: do not call |read| when there is no input available.

I noticed the problem when I was checking my local patches for bug 1170606.

|ReadChar()| routine calls read even when the stream is at the end.
Strangely it even tries to call |Available()| to see if there is any data available for read.
It seems there once was a trial to make this routine use buffered read.

For now, I propose that we don't call |read()| if there is no input available.

Try job is
Aside from some common bugs(?), I don't see any ill effect.

TIA

Comment 1

[ISHIKAWA, Chiaki]

Attachment: fix-readchar-rev02.patch: do not call |read| when there is no input available. Adjusted for directory layout changes, etc.

This works and has been used for several months locally and my tryserver jobs.
This has taken care of the directory layout changes, etc.

I have moved it to near the top of my patch queue to land it.

So requesting review.

Comment 2

[ISHIKAWA, Chiaki]

Comment on attachment 9188698 [details] [diff] [review]
fix-readchar-rev02.patch: do not call |read| when there is no input available. Adjusted for directory layout changes, etc.

I picked the reviewer without any review queue.

Comment 3

[ISHIKAWA, Chiaki]

Not calling read() was useful to analyze short read issues also.
https://bugzilla.mozilla.org/show_bug.cgi?id=1170606

Comment 4

[ISHIKAWA, Chiaki]

It seems that Firefox M-C uses a new submission tool, so I would like to confirm it.

For TB, I can simply add the r+=darktrojan to the message line to the patch, and then set checkin-needed-tb keyword?
(It would be super if we can simply edit the attachment to add the r+=darktrojan string and done with it. Maybe I am missing a tool to facilitate this.)
I will do this later today. This is morning in Japan.

Comment 5

[Geoff Lankow (:darktrojan)]

Just set checkin-needed, the person doing the landing can set the reviewer. Chances are it'll be me anyway.

Comment 6

[ISHIKAWA, Chiaki]

(In reply to Geoff Lankow (:darktrojan) from comment #5)

Just set checkin-needed, the person doing the landing can set the reviewer. Chances are it'll be me anyway.

So that means I don't have to upload the patch with the modified message line. Wow, I always thought that was a drag. Maybe I misunderstood the procedure all along. (!).

I set the checkin-needed-tb flag.

Thank you.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/4c8d82dc0948
Fix ReadChar to not call read when there is no input is available. r=darktrojan