Re: [patch] basename vs. __gnu_basename fix

[Corinna Vinschen <vinschen at redhat dot com>]

On Apr 22 13:07, Craig Howland wrote:
> On 04/22/2015 12:54 PM, Corinna Vinschen wrote:
> >On Apr 22 09:51, Craig Howland wrote:
> >>
> >>On 04/22/2015 04:00 AM, Corinna Vinschen wrote:
> >>>On Apr 21 17:07, Craig Howland wrote:
> >>>>On 04/21/2015 11:50 AM, Corinna Vinschen wrote:
> >>>>>[...]
> >>>>>+/* There are two common basename variants.  If you #include <libgen.h> you get
> >>>>>+   the POSIX version; otherwise, if you define _GNU_SOURCE, you get the GNU
> >>>>>+   version via <string.h>.  POSIX requires that #undef basename will still let
> >>>>>+   you invoke the underlying function.  However, this also implies that the
> >>>>>+   POSIX version is used in this case.  That's made sure here. */
> >>>>>+#if __GNU_VISIBLE && !defined(basename)
> >>>>>  char	*_EXFUN(__gnu_basename,(const char *));
> >>>>>  # define basename __gnu_basename
> >>>>>-# endif
> >>>>>  #endif
> >>>>The prototype should not be skipped if basename is defined (which I'm
> >>>>guessing is an unintended change).
> >>>Hang on, the prototype *must* be skipped if basename is defined.  If you
> >>>don't do that you end up with the exact problem my patch is trying to
> >>>fix:  You'll get two contradicting prototypes for basename, one from
> >>>libgen.h, one from string.h.  If basename is defined, it's from
> >>>libgen.h, so basename is already prototyped.  Have a look into the glibc
> >>>headers.
> >>>
> >>No, the prototype is for __gnu_basename--it does not have to be skipped.
> >>The only way in which it is linked to basename is when the #define is
> >>made--which maps basename to __gnu_basename (not vice versa).  The define
> >>must be skipped.
> >Right, it does not necessarily have to be skipped.  But what's the
> >objective?  Nobody should use the function __gnu_basename under that
> >name, and if basename is already defined, the "other" basename function
> >is supposed to be used.
> >
>      Why should nobody use __gnu_basename under that name?  While I'm not
> saying that I would recommend it, it would be a valid way for someone to get
> exactly what they wanted without needing to worry about the vagaries of
> things being mapped.
>      The objective is to keep from changing how things worked before.  (We
> often to to great lengths to keep binary compatibility, for example--of
> which this is not a case, but the analogy is valid.)  If someone is using
> __gnu_basename() directly, which was possible before,

Only if _BASENAME_DEFINED wasn't defined, which implied that it wasn't
available if libgen.h was included before string.h.

If you want to stick to backward compat, you'd have to take this into
account, even if this change is not even a month old.

> we should preserve
> that unless there is a reason that it should be changed.  So to turn the
> question around, what is the objective in changing it so that the
> __gnu_basename prototype would not be visible under all conditions?  (If
> this were being written new, the conversation would be quite different.)
>         Craig

The objective of this patch was trying to do it closer to what glibc
does.  In glibc, the basename prototype in string.h is not available if
basename is #define'd either.

If that doesn't matter, I'm ok with changing it.  Anybody having a
strong opinion here?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgpU8wLW5edLQ.pgp
Description: PGP signature