[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: buffer write bug in mg



> Kjell Wooding wrote:
> > While I'm here, use basename() instead of some custom stuff
> 
> Building on Linux resulted in this warning:
> 
>   buffer.c:626: warning: passing arg 1 of `__xpg_basename' discards qualifiers from pointer target type

Ah right, we've hit this before.

> >From basename(3):
> 
>   CAVEATS
>        basename() returns a pointer to internal static storage space that will
>        be overwritten by subsequent calls.

That's the openbsd caveat. It's okay here, as we do an immediate copy.

The one you probably mean is the one that says "Both dirname and
basename may modify the contents of path, so copies should be passed
to these functions." which is specific to the linux POSIX basename
(which you get when you include libgen)

If you don't include libgen, you should get the glib version, which
doesn't have this little evil behavior.

> So I'd like to suggest this extra patch:

Bleah. That would mean extra strdups wherever baseame and dirname are
used. This isn't needed under OpenBSD, and you should be able to avoid
it by avoiding the aforementioned include.

Does POSIX *actually* insist that basename and dirname MUST have
evil side effects?

> 
> --- /usr/src/usr.bin/mg/buffer.c        Tue Apr  4 11:58:14 2006
> +++ buffer.c    Tue Apr  4 11:52:40 2006
> @@ -622,8 +622,11 @@ baugname(char *bn, const char *fn, size_
>  {
>         int      count;
>         size_t   remain, len;
> +       char    *fn_dup;
>  
> -       len = strlcpy(bn, basename(fn), bs);
> +       fn_dup = strdup(fn);
> +       len = strlcpy(bn, basename(fn_dup), bs);
> +       free(fn_dup);
>         if (len >= bs)
>                 return (FALSE);
> 

And, you missed an error check on that strdup!

-kj



Visit your host, monkey.org