Re: [PATCH] mountlist/ptsname_r: leverage AC_HEADER_MAJOR

2016-12-05 Thread Mike Frysinger
On 02 Dec 2016 01:40, Bruno Haible wrote:
> Pádraig Brady wrote in 
> http://lists.gnu.org/archive/html/bug-gnulib/2016-04/msg00022.html
> and pushed in 
> http://lists.gnu.org/archive/html/bug-gnulib/2016-11/msg00116.html:
> 
> > Context in 
> > http://lists.gnu.org/archive/html/bug-gnulib/2016-03/msg00025.html
> > glibc-2.23 and musl now need this change it seems.
> > The patch should be fine to apply though and fixes an immediate issue.
> 
> glibc and musl cannot need a change that only affects the behaviour on
> Solaris, AIX, and OSF/1. => There was no "immediate issue".
> 
> Actually the only benefit of the change is that on Solaris, the code now
> uses  instead of .  is somewhat
> cleaner (doesn't define unrelated old cruft).

i was just grepping for all users and bulk converting them over.  you're
certainly correct in the case of the ptsname_r module, the current code
that calls major/minor isn't used on glibc/musl/etc... systems.
-mike


signature.asc
Description: Digital signature


Re: [PATCH] mountlist/ptsname_r: leverage AC_HEADER_MAJOR

2016-12-01 Thread Bruno Haible
Pádraig Brady wrote in 
http://lists.gnu.org/archive/html/bug-gnulib/2016-04/msg00022.html
and pushed in 
http://lists.gnu.org/archive/html/bug-gnulib/2016-11/msg00116.html:

> Context in http://lists.gnu.org/archive/html/bug-gnulib/2016-03/msg00025.html
> glibc-2.23 and musl now need this change it seems.
> The patch should be fine to apply though and fixes an immediate issue.

glibc and musl cannot need a change that only affects the behaviour on
Solaris, AIX, and OSF/1. => There was no "immediate issue".

Actually the only benefit of the change is that on Solaris, the code now
uses  instead of .  is somewhat
cleaner (doesn't define unrelated old cruft).

Bruno




Re: [PATCH] mountlist/ptsname_r: leverage AC_HEADER_MAJOR

2016-04-16 Thread Mike Frysinger
On 16 Apr 2016 13:20, Pádraig Brady wrote:
> On 16/04/16 11:51, Bruno Haible wrote:
> >> These two modules use makedev/major/minor but don't have explicit
> >> checks for the functions.  Use the existing autoconf macro which
> >> will probe some headers for use and set up some defines.
> >
> > Does this change fix a compilation problem? You did not say so.
> 
> Context in http://lists.gnu.org/archive/html/bug-gnulib/2016-03/msg00025.html
> glibc-2.23 and musl now need this change it seems.

heh, i completely forgot about that thread ;)
-mike


signature.asc
Description: Digital signature


Re: [PATCH] mountlist/ptsname_r: leverage AC_HEADER_MAJOR

2016-04-16 Thread Dmitry V. Levin
On Sat, Apr 16, 2016 at 01:20:32PM +0100, Pádraig Brady wrote:
> On 16/04/16 11:51, Bruno Haible wrote:
> >Hi Mike,
> >
> >>These two modules use makedev/major/minor but don't have explicit
> >>checks for the functions.  Use the existing autoconf macro which
> >>will probe some headers for use and set up some defines.
> >
> >Does this change fix a compilation problem? You did not say so.
> 
> Context in 
> http://lists.gnu.org/archive/html/bug-gnulib/2016-03/msg00025.html
> glibc-2.23 and musl now need this change it seems.
> 
> I suggested a makedev gnulib module in the thread above
> as it's not just the headers that are varying,
> as also some systems use mkdev() and some makedev().
> 
> The patch should be fine to apply though and fixes an immediate issue.

Only mountlist part of the patch is relevant, the ptsname_r part is not
needed.


-- 
ldv


pgptAeVKcpomy.pgp
Description: PGP signature


Re: [PATCH] mountlist/ptsname_r: leverage AC_HEADER_MAJOR

2016-04-16 Thread Pádraig Brady

On 16/04/16 11:51, Bruno Haible wrote:

Hi Mike,


These two modules use makedev/major/minor but don't have explicit
checks for the functions.  Use the existing autoconf macro which
will probe some headers for use and set up some defines.


Does this change fix a compilation problem? You did not say so.


Context in http://lists.gnu.org/archive/html/bug-gnulib/2016-03/msg00025.html
glibc-2.23 and musl now need this change it seems.

I suggested a makedev gnulib module in the thread above
as it's not just the headers that are varying,
as also some systems use mkdev() and some makedev().

The patch should be fine to apply though and fixes an immediate issue.

thanks,
Pádraig



Re: [PATCH] mountlist/ptsname_r: leverage AC_HEADER_MAJOR

2016-04-16 Thread Bruno Haible
Hi Mike,

> These two modules use makedev/major/minor but don't have explicit
> checks for the functions.  Use the existing autoconf macro which
> will probe some headers for use and set up some defines.

Does this change fix a compilation problem? You did not say so.

Does this change make future maintenance easier? I don't think so.
mountlist.c and ptsname_r.c are quite special among Gnulib code:
they have #if sections for each particular platform. For example,
the ptsname_r.c file has
  - a code section for Solaris, that uses major, minor,
  - an #include section for Solaris, that includes the appropriate
header,
  - a code section for AIX and OSF/1, that uses minor,
  - an #include section for AIX and OSF/1, that includes the appropriate
header.
This is all consistent.

Solaris, AIX, OSF/1 won't change much in the future. I don't expect
that they will restructure their header files, in the way actively
developed platforms occasionally do.

There is also little chance that one will want to reuse the code for
Solaris, AIX, OSF/1 for any new platform. (GNU/Solaris? Not in
development.)

Similarly for mountlist.c.

So, all I can see is that your change will make configure times longer,
in particular on glibc and BSD systems, for no real purpose.

We have not split mountlist.c and ptsname_r.c into specific files, one
per platform:
  ptsname_r-solaris.c
  ptsname_r-aix-osf.c
  ptsname_r-generic.c
because we thought that maintenance is easier with a single file. Maybe
we should have done it?

Bruno