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 18:26:55 -0600
From: "A. Wilcox" <awilfox@...lielinux.org>
To: musl@...ts.openwall.com
Subject: Re: The Great Big POSIX Conformance Thread [phase 1]

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


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

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.