New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FILES: Remove unnecessary check #156

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@lslebodn
Contributor

lslebodn commented Feb 16, 2017

"grp_iter->gr_mem" is an array of strings and not just a string.
We tried to compare first string to NULL (acctually '\0')
But after that we iterated over the array to find count of members
and we check for NULL one more time.

Lukas Slebodnik
FILES: Remove unnecessary check
"grp_iter->gr_mem" is an array of strings and not just a string.
We tried to compare first string to NULL (acctually '\0')
But after that we iterated over the array to find count of members
and we check for NULL one more time.
@fidencio

This comment has been minimized.

Show comment
Hide comment
@fidencio

fidencio Feb 16, 2017

Contributor

Go for it.

Contributor

fidencio commented Feb 16, 2017

Go for it.

@fidencio fidencio added the Accepted label Feb 16, 2017

@jhrozek

This comment has been minimized.

Show comment
Hide comment
@jhrozek

jhrozek Feb 20, 2017

Contributor

Well, this check was actually intentional. Please grep for gr_mem in proxy_id.c code. I remember from the distant past that we had issues where libc would pass in a zero-length member name..

To be honest, I haven't been able to reproduce this issue and I don't even remember if the issue in proxy code happened with the nss_files proxy -- so if you prefer to have a cleaner code guarding against conditions that we absolutely know how to trigger, feel free to remove the code. I'm just saying I added the check on purpose.

Contributor

jhrozek commented Feb 20, 2017

Well, this check was actually intentional. Please grep for gr_mem in proxy_id.c code. I remember from the distant past that we had issues where libc would pass in a zero-length member name..

To be honest, I haven't been able to reproduce this issue and I don't even remember if the issue in proxy code happened with the nss_files proxy -- so if you prefer to have a cleaner code guarding against conditions that we absolutely know how to trigger, feel free to remove the code. I'm just saying I added the check on purpose.

@lslebodn

This comment has been minimized.

Show comment
Hide comment
@lslebodn

lslebodn Feb 20, 2017

Contributor
Contributor

lslebodn commented Feb 20, 2017

@jhrozek

This comment has been minimized.

Show comment
Hide comment
@jhrozek

jhrozek Feb 22, 2017

Contributor

No, I meant the one in save_group, but you're right the check for \0 is strange.

Contributor

jhrozek commented Feb 22, 2017

No, I meant the one in save_group, but you're right the check for \0 is strange.

@jhrozek

This comment has been minimized.

Show comment
Hide comment
@jhrozek

jhrozek Feb 22, 2017

Contributor
Contributor

jhrozek commented Feb 22, 2017

@jhrozek jhrozek closed this Feb 22, 2017

@jhrozek jhrozek added the Pushed label Feb 22, 2017

@lslebodn lslebodn deleted the lslebodn:files_warning branch Mar 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment