On Thu, Jan 10 2013, Minchan Kim <minchan@xxxxxxxxxx> wrote:
> getc returns "int" so EOF could be -1 but storing getc's return
> value to char directly makes the vaule to 255 so below condition
> is always false.
Technically, this is implementation defined and I believe on many
systems char is signed thus the loop will end on EOF or byte 255.
Either way, my point is the patch is correct, but the comment is not. ;)
Of course, even better if the function just used fgets(), ie. something
like:
int read_block(char *buf, int buf_size, FILE *fin)
{
char *curr = buf, *const buf_end = buf + buf_size;
while (buf_end - curr > 1 && fgets(curr, buf_end - curr, fin)) {
if (*curr == '\n') /* empty line */
return curr - buf;
curr += strlen(curr);
}
return -1; /* EOF or no space left in buf. */
}
which is much shorter and does not have buffer overflow issues.
> It happens in my ARM system so loop is not ended, then segfaulted.
> This patch fixes it.
>
> *curr = getc(fin); // *curr = 255
> if (*curr == EOF) return -1; // if ( 255 == -1)
>
> Cc: Mel Gorman <mgorman@xxxxxxx>
> Cc: Andy Whitcroft <apw@xxxxxxxxxxxx>
> Cc: Alexander Nyberg <alexn@xxxxxxxxx>
> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> ---
> Documentation/page_owner.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/page_owner.c b/Documentation/page_owner.c
> index f0156e1..b777fb6 100644
> --- a/Documentation/page_owner.c
> +++ b/Documentation/page_owner.c
> @@ -32,12 +32,14 @@ int read_block(char *buf, FILE *fin)
> {
> int ret = 0;
> int hit = 0;
> + int vaule;
> char *curr = buf;
>
> for (;;) {
> - *curr = getc(fin);
> - if (*curr == EOF) return -1;
> + value = getc(fin);
> + if (value == EOF) return -1;
>
> + *curr = value;
> ret++;
> if (*curr == '\n' && hit == 1)
> return ret - 1;
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +----<email/xmpp: mpn@xxxxxxxxxx>--------------ooO--(_)--Ooo--Attachment:
pgpL9s6UPFccA.pgp
Description: PGP signature