Re: pkgsrc RC scripts

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Mon, 19 Oct 2009 15:13:14 +1300 (NZDT)
    From:        Steven Drake <sdrake%xnet.co.nz@localhost>
    Message-ID:  <Pine.LNX.4.64.0910102145070.1531%vqena.qenxr.bet.am@localhost>

   |  {
   |    struct config_variable *var;
   |    FILE *fp;
   | -  char *line, *value;
   | +  char *line, *value, *buf;
   |    size_t len, var_len, i;

buf is never initialised, but is tested for NULL at the end.

But it is also not really needed, it appears to exist just to
handle the case of a line not ending in '\n' (that is, a file
not ending in '\n') If the line from fgetln() contains no \n
I think it can be just ignored... (lines not containing '='
are already just silently ignored, I don't see this as
any worse.)

   | +          value=strchr(line, '=');
   | +          if (value == NULL || *value != '=')
   | +                  goto freebuf;

I don't understand the 
        || *value != '='
part of that test.  How can value possibly not be NULL,
and *value not be '=' ?   Are you just testing for strchr() being broken?
Don't bother...

   | +freebuf:
   | +          if (buf != NULL) {
   | +                  free(buf);
   | +                  buf = NULL;
   | +          } 

That's where buf is tested for NULL (which if it remained would be OK).
The "buf = NULL;" there isn't really useful, as the only way buf can
ever be != NULL (other than due to the lack of init bug) is on the last
line of the file, so you know fgetln() will return NULL next time it is
called, which means this code will never be executed again, and hence buf
is a dead variable after the free(), its value is irrelevant (though this
is harmless to keep) - I'd still get rid of buf completely.

kre

ps: this bit is in the original, not your mods, but I don't think
variable i should be a size_t, a regular int would work fine for it