mailing list of musl libc
 help / color / mirror / code / Atom feed
* The Great Big POSIX Conformance Thread [phase 1]
@ 2017-02-05 23:04 A. Wilcox
  2017-02-06  0:01 ` Rich Felker
  0 siblings, 1 reply; 9+ messages in thread
From: A. Wilcox @ 2017-02-05 23:04 UTC (permalink / raw)
  To: musl

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

Hello fellow muslers,

I have been reading through the source tree of musl for two reasons.
One is to familiarise myself with the codebase more so that I can be
of better use if/when patches need to be written.  The other is doing
a cursory audit of POSIX conformance per XSH 2008 (2016 ed.); we at
Adélie are in the beginning stages of acquiring a license to the
VSX-PCTS2016 test suite (which would be phase 2) and I thought it'd be
prudent to find any obvious errors before obtaining it.

I have found some non-conformant functions through my audit.  Four are
(hopefully) easy to fix; two require more thought; and one will likely
need to be discussed.



Easy fixes
==========

catopen
-------

This function is not implemented, and always fails.  This is [barely]
legal in POSIX, but the failure must set errno.  None of the errors
are very appropriate (unfortunately ENOSYS is invalid); maybe ENOMEM
will work.


getservbyport
-------------

Non-conformance of this function was discussed on IRC.  Rich Felker
had said he would apply the patch I wrote[1], but it has not been
applied yet.  If there is an issue with said patch, please let me know
so that I may fix it.


[1]:
https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-posix-getservbyport.patch


if_nameindex
------------

This function always sets errno to ENOBUFS, even when the function
completes succesfully.  A full test case is available[2], and a
suggested patch is also available[3].


[2]: https://code.foxkit.us/adelie/musl-posix/tree/master/if_nameindex
[3]:
https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-if_nameindex-errno.patch


pathconf
--------

This function does not provide a reference for timestamp resolution.
See the POSIX reference[4] for more information.  This should be easy
to fix.


[4]:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/unistd.h.html#tag_13_77_03_03



Further thought required
========================

confstr
-------

musl returns the empty string ("") for options where it does not have
any values.  The POSIX standard states[5] that if an option is
recognised but does not have a value, errno should not be set and NULL
should be returned.


[5]:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html


getlogin
--------

musl simply returns getenv("LOGNAME").  getlogin(3) is used to
determine the login name of the controlling terminal for the process.
 In addition to getenv("LOGNAME") being improper behaviour (since
getlogin(3) could be used within a su or sudo-run shell), it also
causes both coreutils and busybox logname(1) to break conformance:

```
awilcox on ciall ~ $ su
Password:
ciall ~ # LOGNAME=helloworld logname
helloworld
```

The correct behaviour is shown here on glibc:

```
awilcox on ciall [pts/16 Sun 5 16:51] ~: su
Password:
ciall awilcox # LOGNAME=helloworld logname
awilcox
```

The POSIX standard states that implementations generally check the
terminal of fds 0/1/2, then fall back to /dev/tty.[6]


[6]:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getlogin.html



Discussion required
===================

getdate
-------

The musl implementation of getdate(3)[7] sets getdate_err to 7 for any
of the reasons that it should set to 5 (I/O error while reading
template file - i.e., fgets fails), 7 (there is no line in the
template that matches the input), or 8 (invalid input specification)[8].

While I realise musl will probably never implement all conditions that
could provide error condition 8, I feel that error condition 5 should
at least be implemented.  Something as simple as if (ferror(f))
getdate_err = 5;  else getdate_err = 7;  could suffice, IMO.  However,
it would be nice to see better, more robust error handling for
condition 8 as well.


[7]: http://git.musl-libc.org/cgit/musl/tree/src/time/getdate.c#n32
[8]:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getdate.html



Best regards,
--arw

-- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
http://adelielinux.org


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: The Great Big POSIX Conformance Thread [phase 1]
  2017-02-05 23:04 The Great Big POSIX Conformance Thread [phase 1] A. Wilcox
@ 2017-02-06  0:01 ` Rich Felker
  2017-02-06  0:26   ` A. Wilcox
  2017-02-06 22:25   ` A. Wilcox
  0 siblings, 2 replies; 9+ messages in thread
From: Rich Felker @ 2017-02-06  0:01 UTC (permalink / raw)
  To: musl

On Sun, Feb 05, 2017 at 05:04:43PM -0600, A. Wilcox wrote:
> Hello fellow muslers,
> 
> I have been reading through the source tree of musl for two reasons.
> One is to familiarise myself with the codebase more so that I can be
> of better use if/when patches need to be written.  The other is doing
> a cursory audit of POSIX conformance per XSH 2008 (2016 ed.); we at
> Adélie are in the beginning stages of acquiring a license to the
> VSX-PCTS2016 test suite (which would be phase 2) and I thought it'd be
> prudent to find any obvious errors before obtaining it.
> 
> I have found some non-conformant functions through my audit.  Four are
> (hopefully) easy to fix; two require more thought; and one will likely
> need to be discussed.
> 
> 
> 
> Easy fixes
> ==========
> 
> catopen
> -------
> 
> This function is not implemented, and always fails.  This is [barely]
> legal in POSIX, but the failure must set errno.  None of the errors
> are very appropriate (unfortunately ENOSYS is invalid); maybe ENOMEM
> will work.

For any interface that has standard errors, additional errors are also
valid as long as they don't overlap one of the standard error codes
for the interface. As such, it would not be appropriate to use ENOMEM.
EOPNOTSUPP might be appropriate. Eventually this should be fixed with
a non-stub implementation.

> getservbyport
> -------------
> 
> Non-conformance of this function was discussed on IRC.  Rich Felker
> had said he would apply the patch I wrote[1], but it has not been
> applied yet.  If there is an issue with said patch, please let me know
> so that I may fix it.
> 
> 
> [1]:
> https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-posix-getservbyport.patch

This patch probably needs to be checked better still. Knowing the
legaycy get*by*_r interfaces, I suspect it's invalid to return without
setting *res to something. Also it might be better to just check
earlier (at the top) if the argument parses as a number, and bail
immediately, rather than first doing all the work then throwing the
result away.

> if_nameindex
> ------------
> 
> This function always sets errno to ENOBUFS, even when the function
> completes succesfully.  A full test case is available[2], and a
> suggested patch is also available[3].
> 
> 
> [2]: https://code.foxkit.us/adelie/musl-posix/tree/master/if_nameindex
> [3]:
> https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-if_nameindex-errno.patch

There's nothing wrong about this. Any interface that's not explicitly
documented to preserve errno on success (only a few of these exist) is
free to clobber it with any nonzero value.

> pathconf
> --------
> 
> This function does not provide a reference for timestamp resolution.
> See the POSIX reference[4] for more information.  This should be easy
> to fix.
> 
> 
> [4]:
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/unistd.h.html#tag_13_77_03_03

  If the following symbolic constants are defined in the <unistd.h>
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  header, they apply to files and all paths in all file systems on the
  implementation:

I suspect it's intentionaly omitted because the resolution may vary by
pathname. Are you saying we should support _POSIX_TIMESTAMP_RESOLUTION
to get a per-pathname result? Do you know the correct way to compute
it?

> Further thought required
> ========================
> 
> confstr
> -------
> 
> musl returns the empty string ("") for options where it does not have
> any values.
> 
> The POSIX standard states[5] that if an option is
> recognised but does not have a value, errno should not be set and NULL
> should be returned.
> 
> 
> [5]:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html

Can you clarify which _CS_* values you're saying it doesn't have any
values for? For many of them (all that make sense but _CS_PATH) it
does have a value, the empty string. It looks like you might be right
that some of them are non-supported options and should return a null
pointer and leave errno unchanged (this is one of the few functions
explicitly documented not to change errno in that case).

> getlogin
> --------
> 
> musl simply returns getenv("LOGNAME").  getlogin(3) is used to
> determine the login name of the controlling terminal for the process.
>  In addition to getenv("LOGNAME") being improper behaviour (since
> getlogin(3) could be used within a su or sudo-run shell), it also
> causes both coreutils and busybox logname(1) to break conformance:

What do you expect, vs what happens, in the case of su/sudo?

> ```
> awilcox on ciall ~ $ su
> Password:
> ciall ~ # LOGNAME=helloworld logname
> helloworld
> ```
> 
> The correct behaviour is shown here on glibc:
> 
> ```
> awilcox on ciall [pts/16 Sun 5 16:51] ~: su
> Password:
> ciall awilcox # LOGNAME=helloworld logname
> awilcox
> ```
> 
> The POSIX standard states that implementations generally check the
> terminal of fds 0/1/2, then fall back to /dev/tty.[6]
> 
> 
> [6]:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getlogin.html

This one has been discussed and I have a mostly-done proposed function
that can determine the owner of the controlling tty (dependent of
course on /proc).

> Discussion required
> ===================
> 
> getdate
> -------
> 
> The musl implementation of getdate(3)[7] sets getdate_err to 7 for any
> of the reasons that it should set to 5 (I/O error while reading
> template file - i.e., fgets fails), 7 (there is no line in the
> template that matches the input), or 8 (invalid input specification)[8].
> 
> While I realise musl will probably never implement all conditions that
> could provide error condition 8, I feel that error condition 5 should
> at least be implemented.  Something as simple as if (ferror(f))
> getdate_err = 5;  else getdate_err = 7;  could suffice, IMO.  However,
> it would be nice to see better, more robust error handling for
> condition 8 as well.
> 
> 
> [7]: http://git.musl-libc.org/cgit/musl/tree/src/time/getdate.c#n32
> [8]:
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getdate.html

This all seems reasonable. I think error 8 really only makes sense for
implementations that go through an intermediate time_t as part of the
processing. Otherwise there is no way to ascertain whether an input
specification is valid without having it match one of the formats.

Rich


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: The Great Big POSIX Conformance Thread [phase 1]
  2017-02-06  0:01 ` Rich Felker
@ 2017-02-06  0:26   ` A. Wilcox
  2017-02-06 22:25   ` A. Wilcox
  1 sibling, 0 replies; 9+ messages in thread
From: A. Wilcox @ 2017-02-06  0:26 UTC (permalink / raw)
  To: musl

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

On 05/02/17 18:01, Rich Felker wrote:
> On Sun, Feb 05, 2017 at 05:04:43PM -0600, A. Wilcox wrote:
>> catopen -------
>> 
>> This function is not implemented, and always fails.  This is
>> [barely] legal in POSIX, but the failure must set errno.  None of
>> the errors are very appropriate (unfortunately ENOSYS is
>> invalid); maybe ENOMEM will work.
> 
> For any interface that has standard errors, additional errors are
> also valid as long as they don't overlap one of the standard error
> codes for the interface. As such, it would not be appropriate to
> use ENOMEM. EOPNOTSUPP might be appropriate. Eventually this should
> be fixed with a non-stub implementation.


Sounds good to me.


>> getservbyport -------------
>> 
>> Non-conformance of this function was discussed on IRC.  Rich
>> Felker had said he would apply the patch I wrote[1], but it has
>> not been applied yet.  If there is an issue with said patch,
>> please let me know so that I may fix it.
>> 
>> 
>> [1]: 
>> https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-posix-getservbyport.patch
>
>> 
> This patch probably needs to be checked better still. Knowing the 
> legaycy get*by*_r interfaces, I suspect it's invalid to return
> without setting *res to something. Also it might be better to just
> check earlier (at the top) if the argument parses as a number, and
> bail immediately, rather than first doing all the work then
> throwing the result away.


getservbyport_r is not specified in POSIX.

The problem is that musl uses getnameinfo which returns the number if
nothing matches.  getservbyport is specifically supposed to return
NULL if nothing matches.  Since getservbyport calls into
getservbyport_r, I was able to fix its behaviour in addition to
getservbyport with the single patch.


>> if_nameindex ------------
>> 
>> This function always sets errno to ENOBUFS, even when the
>> function completes succesfully.  A full test case is
>> available[2], and a suggested patch is also available[3].
>> 
>> 
>> [2]:
>> https://code.foxkit.us/adelie/musl-posix/tree/master/if_nameindex
>>
>> 
[3]:
>> https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-if_nameindex-errno.patch
>
>> 
> There's nothing wrong about this. Any interface that's not
> explicitly documented to preserve errno on success (only a few of
> these exist) is free to clobber it with any nonzero value.


You are indeed correct; this could still be an effort to be cleaner
with errno.  If you do not want the patch, it should be fine.


>> pathconf --------
>> 
>> This function does not provide a reference for timestamp
>> resolution. See the POSIX reference[4] for more information.
>> This should be easy to fix.
>> 
>> 
>> [4]: 
>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/unistd.h.html#tag_13_77_03_03
>
>> 
> If the following symbolic constants are defined in the <unistd.h> 
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ header, they apply
> to files and all paths in all file systems on the implementation:
> 
> I suspect it's intentionaly omitted because the resolution may vary
> by pathname. Are you saying we should support
> _POSIX_TIMESTAMP_RESOLUTION to get a per-pathname result? Do you
> know the correct way to compute it?


I do not know the correct way to compute it and I will do further
investigation.  It is not legal to omit it[1].


[1]:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/pathconf.html


>> confstr -------
>> 
>> musl returns the empty string ("") for options where it does not
>> have any values.
>> 
>> The POSIX standard states[5] that if an option is recognised but
>> does not have a value, errno should not be set and NULL should be
>> returned.
>> 
>> 
>> [5]: 
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html
>
>> 
> Can you clarify which _CS_* values you're saying it doesn't have
> any values for? For many of them (all that make sense but _CS_PATH)
> it does have a value, the empty string. It looks like you might be
> right that some of them are non-supported options and should return
> a null pointer and leave errno unchanged (this is one of the few
> functions explicitly documented not to change errno in that case).


I do not believe musl should define _CS_POSIX_V7_WIDTH_RESTRICTED_ENVS.


musl sysconf.c:
		[_SC_V7_ILP32_OFF32] = -1,

means these should be NULL:

_CS_POSIX_V7_ILP32_OFF32_CFLAGS
_CS_POSIX_V7_ILP32_OFF32_LDFLAGS
_CS_POSIX_V7_ILP32_OFF32_LIBS


musl sysconf.c:
		[_SC_V7_LPBIG_OFFBIG] = -1,

means these should be NULL:

_CS_POSIX_V7_LPBIG_OFFBIG_CFLAGS
_CS_POSIX_V7_LPBIG_OFFBIG_LDFLAGS
_CS_POSIX_V7_LPBIG_OFFBIG_LIBS


>> getlogin --------
>> 
>> musl simply returns getenv("LOGNAME").  getlogin(3) is used to 
>> determine the login name of the controlling terminal for the
>> process. In addition to getenv("LOGNAME") being improper
>> behaviour (since getlogin(3) could be used within a su or
>> sudo-run shell), it also causes both coreutils and busybox
>> logname(1) to break conformance:
> 
> What do you expect, vs what happens, in the case of su/sudo?


sudo -i and su -/-l both set LOGNAME.  That means running 'logname'
that uses getlogin(3) on musl fails to return the correct value.


> 
>> ``` awilcox on ciall ~ $ su Password: ciall ~ #
>> LOGNAME=helloworld logname helloworld ```
>> 
>> The correct behaviour is shown here on glibc:
>> 
>> ``` awilcox on ciall [pts/16 Sun 5 16:51] ~: su Password: ciall
>> awilcox # LOGNAME=helloworld logname awilcox ```
>> 
>> The POSIX standard states that implementations generally check
>> the terminal of fds 0/1/2, then fall back to /dev/tty.[6]
>> 
>> 
>> [6]: 
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getlogin.html
>
>> 
> This one has been discussed and I have a mostly-done proposed
> function that can determine the owner of the controlling tty
> (dependent of course on /proc).


Good to hear.


>> Discussion required ===================
>> 
>> getdate -------
>> 
>> The musl implementation of getdate(3)[7] sets getdate_err to 7
>> for any of the reasons that it should set to 5 (I/O error while
>> reading template file - i.e., fgets fails), 7 (there is no line
>> in the template that matches the input), or 8 (invalid input
>> specification)[8].
>> 
>> While I realise musl will probably never implement all conditions
>> that could provide error condition 8, I feel that error condition
>> 5 should at least be implemented.  Something as simple as if
>> (ferror(f)) getdate_err = 5;  else getdate_err = 7;  could
>> suffice, IMO.  However, it would be nice to see better, more
>> robust error handling for condition 8 as well.
>> 
>> 
>> [7]:
>> http://git.musl-libc.org/cgit/musl/tree/src/time/getdate.c#n32 
>> [8]: 
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getdate.html
>
>> 
> This all seems reasonable. I think error 8 really only makes sense
> for implementations that go through an intermediate time_t as part
> of the processing. Otherwise there is no way to ascertain whether
> an input specification is valid without having it match one of the
> formats.


When I send the timestamp resolution patch I can send this too if you
like.


> 
> Rich
> 


Best,
--arw

-- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
http://adelielinux.org


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: The Great Big POSIX Conformance Thread [phase 1]
  2017-02-06  0:01 ` Rich Felker
  2017-02-06  0:26   ` A. Wilcox
@ 2017-02-06 22:25   ` A. Wilcox
  2017-06-09  5:26     ` POSIX conformance patches (phase 1) A. Wilcox
  1 sibling, 1 reply; 9+ messages in thread
From: A. Wilcox @ 2017-02-06 22:25 UTC (permalink / raw)
  To: musl


[-- Attachment #1.1: Type: text/plain, Size: 1435 bytes --]

On 05/02/17 18:01, Rich Felker wrote:
> On Sun, Feb 05, 2017 at 05:04:43PM -0600, A. Wilcox wrote:
>> getservbyport
>> -------------
>>
>> Non-conformance of this function was discussed on IRC.  Rich Felker
>> had said he would apply the patch I wrote[1], but it has not been
>> applied yet.  If there is an issue with said patch, please let me know
>> so that I may fix it.
>>
>>
>> [1]:
>> https://code.foxkit.us/adelie/patches/raw/master/sys-libs/musl/musl-1.1.15-posix-getservbyport.patch
> 
> This patch probably needs to be checked better still. Knowing the
> legaycy get*by*_r interfaces, I suspect it's invalid to return without
> setting *res to something. Also it might be better to just check
> earlier (at the top) if the argument parses as a number, and bail
> immediately, rather than first doing all the work then throwing the
> result away.


In addition to my earlier comments in my last mail, I have found a
standard definition for getservbyport_r[1] and it states that on error,
*res shall be set to NULL.  The function does this at the start[2], so I
think this patch should be conformant.

[1]:
http://refspecs.linux-foundation.org/LSB_5.0.0/LSB-Core-generic/LSB-Core-generic/baselib-getservbyport-r.html
[2]:
http://git.musl-libc.org/cgit/musl/tree/src/network/getservbyport_r.c#n23


Best,
--arw

-- 
A. Wilcox (awilfox)
Project Lead, Adélie Linux
http://adelielinux.org

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-getservbyport_r-return-ENOENT-if-service-not-found.patch --]
[-- Type: text/x-patch; name="0001-getservbyport_r-return-ENOENT-if-service-not-found.patch", Size: 1572 bytes --]

From 3579f8840d48f7cabc303d480673d5de72b3c757 Mon Sep 17 00:00:00 2001
From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>
Date: Mon, 6 Feb 2017 16:12:21 -0600
Subject: [PATCH] getservbyport_r: return ENOENT if service not found

getnameinfo will return the numeric form of the service if it has no
name.  This is conformant behaviour for getnameinfo, but not for
getservbyport{,_r}.

NFS for Linux and some BitTorrent clients use getservbyport(3) to check
if a port has an IANA reservation before using it for port mapping.
With the present behaviour, these packages (and assumedly others) fail
to ever find a free port, and refuse to start (or in the case of NFS,
simply refuse any connections).

This patch solves the conformance issue for getservbyport *and*
getservbyport_r (since the first calls in to the second in musl).

Signed-off-by: A. Wilcox <AWilcox@Wilcox-Tech.com>
---
 src/network/getservbyport_r.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/network/getservbyport_r.c b/src/network/getservbyport_r.c
index a0a7cae..37a001b 100644
--- a/src/network/getservbyport_r.c
+++ b/src/network/getservbyport_r.c
@@ -5,6 +5,7 @@
 #include <inttypes.h>
 #include <errno.h>
 #include <string.h>
+#include <stdlib.h>
 
 int getservbyport_r(int port, const char *prots,
 	struct servent *se, char *buf, size_t buflen, struct servent **res)
@@ -50,6 +51,7 @@ int getservbyport_r(int port, const char *prots,
 		break;
 	}
 
+	if (strtol(buf, 0, 10)==ntohs(port)) return ENOENT;
 	*res = se;
 	return 0;
 }
-- 
2.10.0


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* POSIX conformance patches (phase 1)
  2017-02-06 22:25   ` A. Wilcox
@ 2017-06-09  5:26     ` A. Wilcox
  2017-06-09  5:26       ` [PATCH 1/3] catopen: set errno to EOPNOTSUPP A. Wilcox
                         ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: A. Wilcox @ 2017-06-09  5:26 UTC (permalink / raw)
  To: musl

I know this is a long time coming, but as promised in February this thread
contains three patches to correct deviances to the POSIX standard that I
found in musl.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] catopen: set errno to EOPNOTSUPP
  2017-06-09  5:26     ` POSIX conformance patches (phase 1) A. Wilcox
@ 2017-06-09  5:26       ` A. Wilcox
  2017-06-09  5:26       ` [PATCH 2/3] confstr: don't give empty strings for unset values A. Wilcox
  2017-06-09  5:26       ` [PATCH 3/3] getdate: correctly specify error number A. Wilcox
  2 siblings, 0 replies; 9+ messages in thread
From: A. Wilcox @ 2017-06-09  5:26 UTC (permalink / raw)
  To: musl; +Cc: A. Wilcox

From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>

Per 1003.1-2008 (2016 ed.), catopen must set errno on failure.

We set errno to EOPNOTSUPP because musl does not currently support
message catalogues.
---
 src/locale/catopen.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/locale/catopen.c b/src/locale/catopen.c
index 4423c4d..3fbc771 100644
--- a/src/locale/catopen.c
+++ b/src/locale/catopen.c
@@ -1,6 +1,8 @@
 #include <nl_types.h>
+#include <errno.h>
 
 nl_catd catopen (const char *name, int oflag)
 {
+	errno = EOPNOTSUPP;
 	return (nl_catd)-1;
 }
-- 
2.10.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/3] confstr: don't give empty strings for unset values
  2017-06-09  5:26     ` POSIX conformance patches (phase 1) A. Wilcox
  2017-06-09  5:26       ` [PATCH 1/3] catopen: set errno to EOPNOTSUPP A. Wilcox
@ 2017-06-09  5:26       ` A. Wilcox
  2017-06-15  0:00         ` Rich Felker
  2017-06-09  5:26       ` [PATCH 3/3] getdate: correctly specify error number A. Wilcox
  2 siblings, 1 reply; 9+ messages in thread
From: A. Wilcox @ 2017-06-09  5:26 UTC (permalink / raw)
  To: musl; +Cc: A. Wilcox

From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>

If confstr(3) returns an empty string, that means the implementation
supports the configuration and simply needs no setting.  Some values
should actually be unset on musl.  This change ensures that
configurations not supported by musl do not have values.
---
 src/conf/confstr.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/conf/confstr.c b/src/conf/confstr.c
index 02cb1aa..c899abd 100644
--- a/src/conf/confstr.c
+++ b/src/conf/confstr.c
@@ -11,6 +11,19 @@ size_t confstr(int name, char *buf, size_t len)
 		errno = EINVAL;
 		return 0;
 	}
+
+	if (name == _CS_POSIX_V7_WIDTH_RESTRICTED_ENVS ||
+	    // Since sysconf _SC_V7_ILP32_OFF32 = -1
+	    name == _CS_POSIX_V7_ILP32_OFF32_CFLAGS ||
+	    name == _CS_POSIX_V7_ILP32_OFF32_LDFLAGS ||
+	    name == _CS_POSIX_V7_ILP32_OFF32_LIBS ||
+	    // Since sysconf _SC_V7_LPBIG_OFFBIG = -1
+	    name == _CS_POSIX_V7_LPBIG_OFFBIG_CFLAGS ||
+	    name == _CS_POSIX_V7_LPBIG_OFFBIG_LDFLAGS ||
+	    name == _CS_POSIX_V7_LPBIG_OFFBIG_LIBS) {
+		return 0;
+	}
+
 	// snprintf is overkill but avoid wasting code size to implement
 	// this completely useless function and its truncation semantics
 	return snprintf(buf, len, "%s", s) + 1;
-- 
2.10.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 3/3] getdate: correctly specify error number
  2017-06-09  5:26     ` POSIX conformance patches (phase 1) A. Wilcox
  2017-06-09  5:26       ` [PATCH 1/3] catopen: set errno to EOPNOTSUPP A. Wilcox
  2017-06-09  5:26       ` [PATCH 2/3] confstr: don't give empty strings for unset values A. Wilcox
@ 2017-06-09  5:26       ` A. Wilcox
  2 siblings, 0 replies; 9+ messages in thread
From: A. Wilcox @ 2017-06-09  5:26 UTC (permalink / raw)
  To: musl; +Cc: A. Wilcox

From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>

POSIX defines getdate error #5 as:
"An I/O error is encountered while reading the template file."

POSIX defines getdate error #7 as:
"There is no line in the template that matches the input."

This change correctly disambiguates between the two error conditions.
---
 src/time/getdate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/time/getdate.c b/src/time/getdate.c
index 89f2169..420cd8e 100644
--- a/src/time/getdate.c
+++ b/src/time/getdate.c
@@ -37,7 +37,8 @@ struct tm *getdate(const char *s)
 		}
 	}
 
-	getdate_err = 7;
+	if (ferror(f)) getdate_err = 5;
+	else getdate_err = 7;
 out:
 	if (f) fclose(f);
 	pthread_setcancelstate(cs, 0);
-- 
2.10.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/3] confstr: don't give empty strings for unset values
  2017-06-09  5:26       ` [PATCH 2/3] confstr: don't give empty strings for unset values A. Wilcox
@ 2017-06-15  0:00         ` Rich Felker
  0 siblings, 0 replies; 9+ messages in thread
From: Rich Felker @ 2017-06-15  0:00 UTC (permalink / raw)
  To: musl

On Fri, Jun 09, 2017 at 12:26:17AM -0500, A. Wilcox wrote:
> From: "A. Wilcox" <AWilcox@Wilcox-Tech.com>
> 
> If confstr(3) returns an empty string, that means the implementation
> supports the configuration and simply needs no setting.  Some values
> should actually be unset on musl.  This change ensures that
> configurations not supported by musl do not have values.
> ---
>  src/conf/confstr.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/conf/confstr.c b/src/conf/confstr.c
> index 02cb1aa..c899abd 100644
> --- a/src/conf/confstr.c
> +++ b/src/conf/confstr.c
> @@ -11,6 +11,19 @@ size_t confstr(int name, char *buf, size_t len)
>  		errno = EINVAL;
>  		return 0;
>  	}
> +
> +	if (name == _CS_POSIX_V7_WIDTH_RESTRICTED_ENVS ||
> +	    // Since sysconf _SC_V7_ILP32_OFF32 = -1
> +	    name == _CS_POSIX_V7_ILP32_OFF32_CFLAGS ||
> +	    name == _CS_POSIX_V7_ILP32_OFF32_LDFLAGS ||
> +	    name == _CS_POSIX_V7_ILP32_OFF32_LIBS ||
> +	    // Since sysconf _SC_V7_LPBIG_OFFBIG = -1
> +	    name == _CS_POSIX_V7_LPBIG_OFFBIG_CFLAGS ||
> +	    name == _CS_POSIX_V7_LPBIG_OFFBIG_LDFLAGS ||
> +	    name == _CS_POSIX_V7_LPBIG_OFFBIG_LIBS) {
> +		return 0;
> +	}

I'm not sure musl is actually correct in reporting these right now.
POSIX describes LPBIG_OFFBIG as long, pointer, and off_t being "at
least 64 bits", which is the case on 64-bit archs. Of course
ILP32_OFF32 is always false. But I don't think POSIX imposes any
requirement that we return an error here anyway. See unistd.h:

_CS_POSIX_V7_ILP32_OFF32_CFLAGS
    If sysconf(_SC_V7_ILP32_OFF32) returns -1, the meaning of this
    value is unspecified.
             ^^^^^^^^^^^

Also I'm not sure what makes sense to return for
_CS_POSIX_V7_WIDTH_RESTRICTED_ENVS. On most archs, the only
compilation environment does satisfy the conditions, but it doesn't
have a name that can be passed to getconf -v. On some (x32, maybe
n32?) archs, strictly speaking we're nonconforming by not having such
an environment (I think at least suseconds_t is wrong, maybe blksize_t
too). The requirements for interaction with POSIX.2 (xcu c99 and
getconf commands) make this whole area a mess to implement since we
don't control that part of the implementation.

BTW your other two patches look fine, committing them.

Rich


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-06-15  0:00 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-05 23:04 The Great Big POSIX Conformance Thread [phase 1] A. Wilcox
2017-02-06  0:01 ` Rich Felker
2017-02-06  0:26   ` A. Wilcox
2017-02-06 22:25   ` A. Wilcox
2017-06-09  5:26     ` POSIX conformance patches (phase 1) A. Wilcox
2017-06-09  5:26       ` [PATCH 1/3] catopen: set errno to EOPNOTSUPP A. Wilcox
2017-06-09  5:26       ` [PATCH 2/3] confstr: don't give empty strings for unset values A. Wilcox
2017-06-15  0:00         ` Rich Felker
2017-06-09  5:26       ` [PATCH 3/3] getdate: correctly specify error number A. Wilcox

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).