Re: [PATCH 1/3] Add the ``--mount'' command line option.

[Sergiu Ivanov]

Hello,

On Fri, Aug 07, 2009 at 09:30:22PM +0200, olafBuddenhagen@gmx.net wrote:
> On Mon, Aug 03, 2009 at 08:41:15PM +0300, Sergiu Ivanov wrote:
> 
> > +      if ((err = asprintf (&buf, "%s=\"%s\"", OPT_LONG (OPT_LONG_MOUNT),
> > +                      mountee_cl)) == -1)
> > +   {
> > +     free (mountee_cl);
> > +     return ENOMEM;
> > +   }
> > +
> > +      err = argz_add (argz, argz_len, buf);
> > +
> > +      free (buf);
> > +      free (mountee_cl);
> 
> You ignored my previous remark: please handle the error condition the same as
> in other parts of the function!

Ah, sorry :-( I didn't ignore it, I just didn't notice that there was
a ``!='' sign in the if :-(
 
> To be more specific, a few lines below we have this code:
> 
>             char *buf = NULL;
>             if ((err = asprintf (&buf, "%s=%s", OPT_LONG (OPT_LONG_PRIORITY),
>                       ulfs->priority)) != -1)
>               {
>                 err = argz_add (argz, argz_len, buf);
>                 free (buf);
>               }
> 
> It does exactly the same -- please don't write it differently.
> 
> (This variant is more concise too, and thus more elegant. It avoids the need
> for the explicit return and additional free().)
 
Yeah, sorry :-( I'll fix it.

Regards,
scolobb