[PATCH] Hide sethostname() in unistd.h

[PATCH] Hide sethostname() in unistd.h

[Christian Franke]

[-- Attachment #1: Type: text/plain, Size: 214 bytes --]

Found during an experimental build of busybox:

The sethostname() prototype in /usr/include/sys/unistd.h is enabled also 
on Cygwin.
It should be disabled because Cygwin does not provide this function.

Christian


[-- Attachment #2: unistd-sethostname.patch --]
[-- Type: text/x-patch, Size: 750 bytes --]

2015-06-16  Christian Franke  <franke@computer.org>

	* libc/include/sys/unistd.h (sethostname): Hide prototype on Cygwin.

diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
index eb26921..6131b5c 100644
--- a/newlib/libc/include/sys/unistd.h
+++ b/newlib/libc/include/sys/unistd.h
@@ -169,7 +169,7 @@ int     _EXFUN(setgid, (gid_t __gid ));
 #if defined(__CYGWIN__)
 int	_EXFUN(setgroups, (int ngroups, const gid_t *grouplist ));
 #endif
-#if __BSD_VISIBLE || (defined(_XOPEN_SOURCE) && __XSI_VISIBLE < 500)
+#if !defined(__CYGWIN__) && (__BSD_VISIBLE || (defined(_XOPEN_SOURCE) && __XSI_VISIBLE < 500))
 int	_EXFUN(sethostname, (const char *, size_t));
 #endif
 int     _EXFUN(setpgid, (pid_t __pid, pid_t __pgid ));

Re: [PATCH] Hide sethostname() in unistd.h

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1618 bytes --]

On Jun 16 18:27, Christian Franke wrote:
> Found during an experimental build of busybox:
> 
> The sethostname() prototype in /usr/include/sys/unistd.h is enabled also on
> Cygwin.
> It should be disabled because Cygwin does not provide this function.
> 
> Christian
> 

> 2015-06-16  Christian Franke  <franke@computer.org>
> 
> 	* libc/include/sys/unistd.h (sethostname): Hide prototype on Cygwin.
> 
> diff --git a/newlib/libc/include/sys/unistd.h b/newlib/libc/include/sys/unistd.h
> index eb26921..6131b5c 100644
> --- a/newlib/libc/include/sys/unistd.h
> +++ b/newlib/libc/include/sys/unistd.h
> @@ -169,7 +169,7 @@ int     _EXFUN(setgid, (gid_t __gid ));
>  #if defined(__CYGWIN__)
>  int	_EXFUN(setgroups, (int ngroups, const gid_t *grouplist ));
>  #endif
> -#if __BSD_VISIBLE || (defined(_XOPEN_SOURCE) && __XSI_VISIBLE < 500)
> +#if !defined(__CYGWIN__) && (__BSD_VISIBLE || (defined(_XOPEN_SOURCE) && __XSI_VISIBLE < 500))
>  int	_EXFUN(sethostname, (const char *, size_t));
>  #endif
>  int     _EXFUN(setpgid, (pid_t __pid, pid_t __pgid ));

What about implementing sethostname instead?

  extern "C" int
  sethostname (const char *name, size_t len)
  {
    WCHAR wname[MAX_COMPUTERNAME_LENGTH + 1];

    sys_mbstowcs (wname, MAX_COMPUTERNAME_LENGTH + 1, name, len);
    if (!SetComputerNameEx (ComputerNamePhysicalDnsHostname, wname))
      {
	__seterrno ();
	return -1;
      }
    return 0;
  }


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Hide sethostname() in unistd.h

[Christian Franke]

Corinna Vinschen wrote:
> On Jun 16 18:27, Christian Franke wrote:
>> Found during an experimental build of busybox:
>>
>> The sethostname() prototype in /usr/include/sys/unistd.h is enabled also on
>> Cygwin.
>> It should be disabled because Cygwin does not provide this function.
>>
>> Christian
>>
>
> What about implementing sethostname instead?
>
>    extern "C" int
>    sethostname (const char *name, size_t len)
> ...

I didn't consider this as an alternative because I guessed that it is 
intentional that sethostname is missing.
(it is not a typical that someone wants to use Cygwin to change the name 
of a Windows machine)

Christian

Re: [PATCH] Hide sethostname() in unistd.h

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On Jun 17 07:38, Christian Franke wrote:
> Corinna Vinschen wrote:
> >On Jun 16 18:27, Christian Franke wrote:
> >>Found during an experimental build of busybox:
> >>
> >>The sethostname() prototype in /usr/include/sys/unistd.h is enabled also on
> >>Cygwin.
> >>It should be disabled because Cygwin does not provide this function.
> >>
> >>Christian
> >>
> >
> >What about implementing sethostname instead?
> >
> >   extern "C" int
> >   sethostname (const char *name, size_t len)
> >...
> 
> I didn't consider this as an alternative because I guessed that it is
> intentional that sethostname is missing.
> (it is not a typical that someone wants to use Cygwin to change the name of
> a Windows machine)

You're right there.  But, we have a lot of interfaces defined in newlib
headers which are not available on all platforms, but we're not
explicitely filtering them per platform.

Afaics, the problem is the configuration of busybox, not unistd.h.
Checking for prototypes in headers is not sufficient to check for the
availablility of functions, only for the availability of the prototype.
The configuration should also try a link check on the function with
AC_CHECK_FUNC or something like that.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Hide sethostname() in unistd.h

[Christian Franke]

Corinna Vinschen wrote:
> On Jun 17 07:38, Christian Franke wrote:
>> Corinna Vinschen wrote:
>>> On Jun 16 18:27, Christian Franke wrote:
>>>> Found during an experimental build of busybox:
>>>>
>>>> The sethostname() prototype in /usr/include/sys/unistd.h is enabled also on
>>>> Cygwin.
>>>> It should be disabled because Cygwin does not provide this function.
>>>>
>>>> Christian
>>>>
>>> What about implementing sethostname instead?
>>>
>>>    extern "C" int
>>>    sethostname (const char *name, size_t len)
>>> ...
>> I didn't consider this as an alternative because I guessed that it is
>> intentional that sethostname is missing.
>> (it is not a typical that someone wants to use Cygwin to change the name of
>> a Windows machine)
> You're right there.  But, we have a lot of interfaces defined in newlib
> headers which are not available on all platforms, but we're not
> explicitely filtering them per platform.

I see. Then let's forget the patch.


> Afaics, the problem is the configuration of busybox, not unistd.h.
> Checking for prototypes in headers is not sufficient to check for the
> availablility of functions, only for the availability of the prototype.
> The configuration should also try a link check on the function with
> AC_CHECK_FUNC or something like that.

Busybox does not use autoconf or similar. It requires manual platform 
specific configuration which does not yet support a missing 
sethostname(). After adding HAVE_SETHOSTNAME manually and some other 
minor additions, busybox (which many commands enabled) compiles and 
works reasonably.
Would ITP make sense ?

Christian

Re: [PATCH] Hide sethostname() in unistd.h

[Yaakov Selkowitz]

On Wed, 2015-06-17 at 22:25 +0200, Christian Franke wrote:
> Busybox does not use autoconf or similar. It requires manual platform 
> specific configuration which does not yet support a missing 
> sethostname(). After adding HAVE_SETHOSTNAME manually and some other 
> minor additions, busybox (which many commands enabled) compiles and 
> works reasonably.
> Would ITP make sense ?

TBH I'm not sure.  Presuming you're discussing the single-executable
build (so as not to clobber coreutils etc.), there is still the question
of (not) matching the heavily-patched coreutils wrt .exe handling etc.
What do you think the use case would be?

--
Yaakov

Re: [PATCH] Hide sethostname() in unistd.h

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1112 bytes --]

On 06/17/2015 02:57 PM, Yaakov Selkowitz wrote:
> On Wed, 2015-06-17 at 22:25 +0200, Christian Franke wrote:
>> Busybox does not use autoconf or similar. It requires manual platform 
>> specific configuration which does not yet support a missing 
>> sethostname(). After adding HAVE_SETHOSTNAME manually and some other 
>> minor additions, busybox (which many commands enabled) compiles and 
>> works reasonably.
>> Would ITP make sense ?
> 
> TBH I'm not sure.  Presuming you're discussing the single-executable
> build (so as not to clobber coreutils etc.), there is still the question
> of (not) matching the heavily-patched coreutils wrt .exe handling etc.
> What do you think the use case would be?

Portability testing is one thing - I often compare how
bash/dash/zsh/mksh handle a shell construct, and adding busybox sh into
the mix adds another perspective.  But yeah, I don't see busybox
becoming the default source of these apps, so much as an alternative
implementation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [PATCH] Hide sethostname() in unistd.h

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1623 bytes --]

On Jun 17 15:15, Eric Blake wrote:
> On 06/17/2015 02:57 PM, Yaakov Selkowitz wrote:
> > On Wed, 2015-06-17 at 22:25 +0200, Christian Franke wrote:
> >> Busybox does not use autoconf or similar. It requires manual platform 
> >> specific configuration which does not yet support a missing 
> >> sethostname(). After adding HAVE_SETHOSTNAME manually and some other 
> >> minor additions, busybox (which many commands enabled) compiles and 
> >> works reasonably.
> >> Would ITP make sense ?
> > 
> > TBH I'm not sure.  Presuming you're discussing the single-executable
> > build (so as not to clobber coreutils etc.), there is still the question
> > of (not) matching the heavily-patched coreutils wrt .exe handling etc.
> > What do you think the use case would be?
> 
> Portability testing is one thing - I often compare how
> bash/dash/zsh/mksh handle a shell construct, and adding busybox sh into
> the mix adds another perspective.  But yeah, I don't see busybox
> becoming the default source of these apps, so much as an alternative
> implementation.

If it's called "busybox" and the package doesn't try to create shortcuts
/bin/sh -> /bin/busybox, etc, I don't see a problem to ITP it.

If those symlinks are required for busybox to work, they should be
encapsulated in their own subdir, something like /usr/libexec/busybox
or so.  Users just need to set $PATH correctly then.  Or maybe that
could be done by busybox as well.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Hide sethostname() in unistd.h

[Christian Franke]

Corinna Vinschen wrote:
> On Jun 17 15:15, Eric Blake wrote:
>> On 06/17/2015 02:57 PM, Yaakov Selkowitz wrote:
>>> On Wed, 2015-06-17 at 22:25 +0200, Christian Franke wrote:
>>>> Busybox does not use autoconf or similar. It requires manual platform
>>>> specific configuration which does not yet support a missing
>>>> sethostname(). After adding HAVE_SETHOSTNAME manually and some other
>>>> minor additions, busybox (which many commands enabled) compiles and
>>>> works reasonably.
>>>> Would ITP make sense ?
>>> TBH I'm not sure.  Presuming you're discussing the single-executable
>>> build (so as not to clobber coreutils etc.), there is still the question
>>> of (not) matching the heavily-patched coreutils wrt .exe handling etc.
>>> What do you think the use case would be?
>> Portability testing is one thing - I often compare how
>> bash/dash/zsh/mksh handle a shell construct, and adding busybox sh into
>> the mix adds another perspective.  But yeah, I don't see busybox
>> becoming the default source of these apps, so much as an alternative
>> implementation.
> If it's called "busybox" and the package doesn't try to create shortcuts
> /bin/sh -> /bin/busybox, etc, I don't see a problem to ITP it.

Symlinks in standard places should not be created, of course.
The shell and other commands could still be started by: busybox COMMAND ..."

> If those symlinks are required for busybox to work, they should be
> encapsulated in their own subdir, something like /usr/libexec/busybox
> or so.  Users just need to set $PATH correctly then.  Or maybe that
> could be done by busybox as well.
Yes: busybox --install -s /some/where

Busybox may occasionally be useful because it provides lightweight 
versions of various commands (including daemons) not part of the Cygwin 
base installation and a few commands not available in any package.

It could also be used to build a minimalistic Cygwin (busybox.exe, 
mintty.exe, cygwin1.dll). If build with standalone option enabled, 
symlinks are not needed then.

Christian

Re: [PATCH] Hide sethostname() in unistd.h

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2317 bytes --]

On Jun 19 07:27, Christian Franke wrote:
> Corinna Vinschen wrote:
> >On Jun 17 15:15, Eric Blake wrote:
> >>On 06/17/2015 02:57 PM, Yaakov Selkowitz wrote:
> >>>On Wed, 2015-06-17 at 22:25 +0200, Christian Franke wrote:
> >>>>Busybox does not use autoconf or similar. It requires manual platform
> >>>>specific configuration which does not yet support a missing
> >>>>sethostname(). After adding HAVE_SETHOSTNAME manually and some other
> >>>>minor additions, busybox (which many commands enabled) compiles and
> >>>>works reasonably.
> >>>>Would ITP make sense ?
> >>>TBH I'm not sure.  Presuming you're discussing the single-executable
> >>>build (so as not to clobber coreutils etc.), there is still the question
> >>>of (not) matching the heavily-patched coreutils wrt .exe handling etc.
> >>>What do you think the use case would be?
> >>Portability testing is one thing - I often compare how
> >>bash/dash/zsh/mksh handle a shell construct, and adding busybox sh into
> >>the mix adds another perspective.  But yeah, I don't see busybox
> >>becoming the default source of these apps, so much as an alternative
> >>implementation.
> >If it's called "busybox" and the package doesn't try to create shortcuts
> >/bin/sh -> /bin/busybox, etc, I don't see a problem to ITP it.
> 
> Symlinks in standard places should not be created, of course.
> The shell and other commands could still be started by: busybox COMMAND ..."
> 
> >If those symlinks are required for busybox to work, they should be
> >encapsulated in their own subdir, something like /usr/libexec/busybox
> >or so.  Users just need to set $PATH correctly then.  Or maybe that
> >could be done by busybox as well.
> Yes: busybox --install -s /some/where
> 
> Busybox may occasionally be useful because it provides lightweight versions
> of various commands (including daemons) not part of the Cygwin base
> installation and a few commands not available in any package.
> 
> It could also be used to build a minimalistic Cygwin (busybox.exe,
> mintty.exe, cygwin1.dll). If build with standalone option enabled, symlinks
> are not needed then.

Neat.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]