-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54952/#review159995
-----------------------------------------------------------
I'm not quite sure the error processing logic of the functions you touch in this patch is
correct. According to [POSIX documentation](http://pubs.opengroup.org/onlinepubs/009695399/functions/getpwnam.html),
`errno` is set only if an error occurs, which does not include absent entries. However this
seems [not to be the case in Linux](https://linux.die.net/man/3/getpwnam_r) and apparently
that's why a special error handling section was introduced in https://reviews.apache.org/r/24249/.
According to the documentation, here is how the workflow should look like (you can group it
into nested `if` statements):
```
if (retcode == 0 && result != nullptr) {
// Matching entry found.
uid_t uid = passwd.pw_uid;
delete[] buffer;
return uid;
} else if (retcode == 0 && result == nullptr) {
// Matching entry not found.
delete[] buffer;
return None();
} else if (retcode != 0 && errno == ERANGE) {
// Buffer is not large enough.
size *= 2;
delete[] buffer;
} else if (retcode != 0 &&
(errno == EINTR ||
errno == EIO ||
errno == EMFILE ||
errno == ENFILE ||
errno == ENOMEM)) {
// Non-transient failure.
delete[] buffer;
return ErrnoError("Failed to get username information");
} else {
// Some Linux distributions...
// Means the entry is not found.
delete[] buffer;
return None();
}
...
```
It seems that fixing the overall workflow is a more robust solution than adding error messages
one by one each time the C library is updated.
- Alexander Rukletsov
On Dec. 21, 2016, 10:38 p.m., Neil Conway wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54952/
> -----------------------------------------------------------
>
> (Updated Dec. 21, 2016, 10:38 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Bugs: MESOS-6826
> https://issues.apache.org/jira/browse/MESOS-6826
>
>
> Repository: mesos
>
>
> Description
> -------
>
> On recent Arch Linux, `getpwnam_r` can return EINVAL when passed an
> invalid user name, which caused the `OsTest.User` test to fail.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/os/posix/su.hpp f3f45ebf006f0f8e7e70b0f4c4ed76465530c58e
>
> Diff: https://reviews.apache.org/r/54952/diff/
>
>
> Testing
> -------
>
> `make check` on Arch Linux. `OsTest.User` fails without this patch but succeeds with
this patch.
>
>
> Thanks,
>
> Neil Conway
>
>
|