svn commit: r303988 - head/lib/libc/gen

[Bryan Drewery]

On 8/24/16 12:49 PM, Ed Schouten wrote:
> 2016-08-24 20:30 GMT+02:00 Bryan Drewery <bdrewery at 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 at FBSD_1.0 and
> dirname at FBSD_1.0.
> 
> According to Bryan, this fixes the problems he was experiencing.
> 
> -- Ed Schouten <ed at 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 at FBSD_1.0
> + * and dirname at 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20160824/883ce99b/attachment.sig>