[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: MInor fixes and problems for malloced getpw/grxxx functions for bb
From:       Denys Vlasenko <vda.linux () googlemail ! com>
Date:       2015-01-06 0:23:46
Message-ID: CAK1hOcNxS+o9N-PSh=09i5_6VCAwZjj+r57j9c64qjj8Bb=qyQ () mail ! gmail ! com
[Download RAW message or body]

Applied, thanks!

On Mon, Jan 5, 2015 at 11:03 PM, tito <farmatito@tiscali.it> wrote:
> On Monday 05 January 2015 15:11:14 you wrote:
>> On Mon, Jan 5, 2015 at 12:14 AM, tito <farmatito@tiscali.it> wrote:
>> > 1) in function parse_common count should start at 1 as line numbers are off by one.
>> >
>> >  static char *parse_common(FILE *fp, struct passdb *db,
>> >                 const char *key, int field_pos)
>> >  {
>> > -       int count = 0;
>> > +       int count = 1;
>> >         char *buf;
>> >
>> >         while ((buf = xmalloc_fgetline(fp)) != NULL) {
>>
>> There is a count++ immediately after we got a line:
>>
>>         int count = 0;
>>         char *buf;
>>         while ((buf = xmalloc_fgetline(fp)) != NULL) {
>>                 count++;
>>
>> > 2) in parse_common the use of key variable seems problematic to me:
>> >
>> >         if (!key) {
>> >                         /* no key specified: sequential read, return a record */
>> >                         break;
>> >         }
>> >
>> > This variable string is passed on from the caller and eventually from the user
>> > we have no guarantee that it will be non NULL when we don't want a sequential read.
>> > If the calling app passes a bad pointer it will get a record returned.
>>
>> I think that getpwnam(NULL) is not a valid use, but ok,
>> we can plug this hole anyway. Switching to field_pos == -1
>> as "no search" indicator.
>>
>>
>> > 3)  in parse_common function
>> >
>> >                while ((buf = xmalloc_fgetline(fp)) != NULL) {
>> >
>> >      does not set errno to ENOENT on EOF
>>
>> How about this simple fix in getXXent_r()? -
>>
>>         buf = parse_common(db->fp, db, /*no search key:*/ NULL, 0);
>> +       if (!buf && !errno)
>> +               errno = ENOENT;
>>         return massage_data_for_r_func(db, buffer, buflen, result, buf);
>>
>>
>> Thanks!
>>
>>
>
> Hi,
> your fixes work great!
> Attached you will find a patch to fix a minor problem:
>
>     remove line count from parse_common because in case of a sequential
>     read the reported bad line is always line 1. (eventually we could print
>     the line but for now we just warn)
>
> Ciao,
> Tito
>
> --- libpwdgrp/pwd_grp.c.original        2015-01-05 21:22:51.000000000 +0100
> +++ libpwdgrp/pwd_grp.c 2015-01-05 22:50:52.851719946 +0100
> @@ -189,17 +189,15 @@ static int tokenize(char *buffer, int ch
>  static char *parse_common(FILE *fp, struct passdb *db,
>                 const char *key, int field_pos)
>  {
> -       int count = 0;
>         char *buf;
>
>         while ((buf = xmalloc_fgetline(fp)) != NULL) {
> -               count++;
>                 /* Skip empty lines, comment lines */
>                 if (buf[0] == '\0' || buf[0] == '#')
>                         goto free_and_next;
>                 if (tokenize(buf, ':') != db->numfields) {
>                         /* number of fields is wrong */
> -                       bb_error_msg("bad record at %s:%u", db->filename, count);
> +                       bb_error_msg("%s: bad record", db->filename);
>                         goto free_and_next;
>                 }
>
>
>
>
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]