Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sun, 5 Feb 2017 19:01:00 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: The Great Big POSIX Conformance Thread [phase 1]

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

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.