On Sat, Jul 01, 2017 at 09:48:08PM +0100, Nicholas Marriott wrote:
> Safer how?

I believe that's in reference to the CAVEATS mentioned in fgetln(3).

http://man.openbsd.org/fgetln.3#CAVEATS

getline(3) returns C strings.

> I don't mind too much, but you will also need to initialize size to 0
> (it is currently uninitialized) before calling getline().
> 
> On Sat, Jul 01, 2017 at 04:35:39PM -0400, Bryan Steele wrote:
> > On Sat, Jul 01, 2017 at 09:27:47PM +0100, Nicholas Marriott wrote:
> > > 
> > > What is the point of doing this? fgetln works fine.
> > > 
> > 
> > There's been other examples of commits replacing fgetln for getline,
> > while portability isn't a major concern, it's been suggested that
> > getline may be safer.
> > 
> > http://marc.info/?l=openbsd-cvs&m=147231601627483&w=2
> > 
> > No worries if you want to keep it.
> > 
> > > 
> > > 
> > > On Sat, Jul 01, 2017 at 02:47:25PM -0400, Bryan Steele wrote:
> > > > Seems to work, does this look right?
> > > > 
> > > > -Bryan.
> > > > 
> > > > Index: magic-load.c
> > > > ===================================================================
> > > > RCS file: /cvs/src/usr.bin/file/magic-load.c,v
> > > > retrieving revision 1.25
> > > > diff -u -p -r1.25 magic-load.c
> > > > --- magic-load.c        1 Jul 2017 14:34:29 -0000       1.25
> > > > +++ magic-load.c        1 Jul 2017 18:44:42 -0000
> > > > @@ -1071,6 +1071,7 @@ magic_load(FILE *f, const char *path, in
> > > >         struct magic_line       *ml = NULL, *parent, *parent0;
> > > >         char                    *line, *tmp;
> > > >         size_t                   size;
> > > > +       ssize_t                  slen;
> > > >         u_int                    at, level, n, i;
> > > >  
> > > >         m = xcalloc(1, sizeof *m);
> > > > @@ -1084,15 +1085,11 @@ magic_load(FILE *f, const char *path, in
> > > >  
> > > >         at = 0;
> > > >         tmp = NULL;
> > > > -       while ((line = fgetln(f, &size))) {
> > > > -               if (line[size - 1] == '\n')
> > > > -                       line[size - 1] = '\0';
> > > > -               else {
> > > > -                       tmp = xmalloc(size + 1);
> > > > -                       memcpy(tmp, line, size);
> > > > -                       tmp[size] = '\0';
> > > > -                       line = tmp;
> > > > -               }
> > > > +       while ((slen = getline(&tmp, &size, f)) != -1) {
> > > > +               line = tmp;
> > > > +               if (line[slen - 1] == '\n')
> > > > +                       line[slen - 1] = '\0';
> > > > +
> > > >                 at++;
> > > >  
> > > >                 while (isspace((u_char)*line))
> > > 

Reply via email to