On 8/24/16 12:49 PM, Ed Schouten wrote: > 2016-08-24 20:30 GMT+02:00 Bryan Drewery <bdrew...@freebsd.org>: >> > That would only fix stable/11, stable/10, stable/9, releng/11.0. >> > >> > It won't fix releng/10.3, releng/10.2, releng/10.1, releng/9.3, etc... >> > without an EN. >> > >> > It won't fix stable/11 - 1, stable/10 - 1, etc. >> > >> > It will never fix releng/8.4 (unsupported releases) since so@ won't EN >> > to those. People do sometimes need to build these older releases still. >> > >> > It creates a line in the sand where we can never build checkouts older >> > than where the fix was at. So I don't think it is the appropriate fix. > Good point! > > Just for the record: Bryan and I just discussed this matter in more > detail on IRC. We came up with a workaround that should be pretty > good. > > Attached is a patch for <libgen.h> that adds some extra logic, so that > any calls to basename() and dirname() will expand to calls to > __old_basename() and __old_dirname(). Using __sym_compat(), these will > cause the compiler to generate calls to basename@FBSD_1.0 and > dirname@FBSD_1.0. > > According to Bryan, this fixes the problems he was experiencing. > > -- Ed Schouten <e...@nuxi.nl> Nuxi, 's-Hertogenbosch, the Netherlands > KvK-nr.: 62051717 > > > dirname-basename-xinstall.diff > > > Index: include/libgen.h > =================================================================== > --- include/libgen.h (revision 304750) > +++ include/libgen.h (working copy) > @@ -39,4 +39,26 @@ > char *dirname(char *); > __END_DECLS > > +/* > + * In FreeBSD 12, the prototype of basename() and dirname() was modified > + * to comply to POSIX. These functions may now modify their input. > + * Unfortunately, our copy of xinstall(8) shipped with previous versions > + * of FreeBSD is built using the host headers and libc during the > + * bootstrapping phase and depends on the old behavior. > + * > + * Apply a workaround where we explicitly link against basename@FBSD_1.0 > + * and dirname@FBSD_1.0 in case these functions are called on constant > + * strings, instead of making the build fail. > + */ > +#if defined(__generic) && !defined(__cplusplus) > +__BEGIN_DECLS > +char *__old_basename(const char *); > +char *__old_dirname(const char *); > +__END_DECLS > +__sym_compat(basename, __old_basename, FBSD_1.0); > +__sym_compat(dirname, __old_dirname, FBSD_1.0); > +#define basename(x) __generic(x, const char *, __old_basename, > basename)(x) > +#define dirname(x) __generic(x, const char *, __old_dirname, > dirname)(x) > +#endif
Personally I really like this in general as an API-compat pattern. I know Ed wanted to deprecate and remove the old dirname(3)/basename(3), but it was quite painful downstream to convert our checkout to use the new prototypes. It spanned "only" 19 files, but it took 2 days of build-and-fix iterations to fix. We still get the benefit of thread-safe basename(3)/dirname(3) when using the proper prototype. Is it possible to cause the use of these old prototypes to print a warning and note that they are deprecated/unsafe? -- Regards, Bryan Drewery
signature.asc
Description: OpenPGP digital signature
