Skip to content
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

Inconsistency between doc and code for chown using negative argument: -1 #15523

Closed
p5pRT opened this issue Aug 16, 2016 · 12 comments
Closed

Inconsistency between doc and code for chown using negative argument: -1 #15523

p5pRT opened this issue Aug 16, 2016 · 12 comments
Labels

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Aug 16, 2016

Migrated from rt.perl.org#128967 (status was 'resolved')

Searchable as RT128967$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 16, 2016

From @atoomic

Recent change to perl 5.24 change the chown behavior when using -1 argument​:
f95ba54

But perldoc for chow still mention​:
"A value of -1 in either position is interpreted by most systems to leave that value unchanged."

I'm not sure which part is incorrect​: the doc ? or the change ?
My code was currently using the chown -1 feature documented.

Is it now deprecated ?
Should we revert this commit or update the doc ?

Thanks for the clarification on this.
nicolas

# chown root​: /tmp/xxx; perl522 -E 'open my $f, ">", "/tmp/xxx"; say chown -1, 99, $f'; ls -l /tmp/xxx
1
-rw-r--r-- 1 root nobody 0 Aug 16 16​:45 /tmp/xxx

# chown root​: /tmp/xxx; perl524 -E 'open my $f, ">", "/tmp/xxx"; say chown -1, 99, $f'; ls -l /tmp/xxx
0
-rw-r--r-- 1 root root 0 Aug 16 16​:46 /tmp/xxx

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 16, 2016

From @cpansprout

On Tue Aug 16 15​:50​:31 2016, atoomic wrote​:

Recent change to perl 5.24 change the chown behavior when using -1
argument​:
f95ba54

But perldoc for chow still mention​:
"A value of -1 in either position is interpreted by most systems to
leave that value unchanged."

I'm not sure which part is incorrect​: the doc ? or the change ?
My code was currently using the chown -1 feature documented.

Is it now deprecated ?
Should we revert this commit or update the doc ?

Thanks for the clarification on this.
nicolas

# chown root​: /tmp/xxx; perl522 -E 'open my $f, ">", "/tmp/xxx"; say
chown -1, 99, $f'; ls -l /tmp/xxx
1
-rw-r--r-- 1 root nobody 0 Aug 16 16​:45 /tmp/xxx

# chown root​: /tmp/xxx; perl524 -E 'open my $f, ">", "/tmp/xxx"; say
chown -1, 99, $f'; ls -l /tmp/xxx
0
-rw-r--r-- 1 root root 0 Aug 16 16​:46 /tmp/xxx

Commit f95ba54 looks like a mistake to me. The change was made to appease a linter that didn’t like the idea of a negative argument because someone decided at some point to make it not like it.

Makes no sense to me.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 16, 2016

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 16, 2016

From @jhi

On Tue Aug 16 15​:59​:52 2016, sprout wrote​:

On Tue Aug 16 15​:50​:31 2016, atoomic wrote​:

Recent change to perl 5.24 change the chown behavior when using -1
argument​:
f95ba54

But perldoc for chow still mention​:
"A value of -1 in either position is interpreted by most systems to
leave that value unchanged."

I'm not sure which part is incorrect​: the doc ? or the change ?
My code was currently using the chown -1 feature documented.

Is it now deprecated ?
Should we revert this commit or update the doc ?

Thanks for the clarification on this.
nicolas

# chown root​: /tmp/xxx; perl522 -E 'open my $f, ">", "/tmp/xxx"; say
chown -1, 99, $f'; ls -l /tmp/xxx
1
-rw-r--r-- 1 root nobody 0 Aug 16 16​:45 /tmp/xxx

# chown root​: /tmp/xxx; perl524 -E 'open my $f, ">", "/tmp/xxx"; say
chown -1, 99, $f'; ls -l /tmp/xxx
0
-rw-r--r-- 1 root root 0 Aug 16 16​:46 /tmp/xxx

Commit f95ba54 looks like a mistake to me. The change was made to
appease a linter that didn’t like the idea of a negative argument
because someone decided at some point to make it not like it.

Makes no sense to me.

Looks like the negative arguments to fchown are supported in some systems, with some semantics, under some conditions. In other words, nothing portable can be said about it. So reverting the change and telling Coverity to shut up about it.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 17, 2016

From @atoomic

Please also revert c4f643b
at the same time you are reverting f95ba54

On Tue Aug 16 16​:09​:54 2016, jhi wrote​:

On Tue Aug 16 15​:59​:52 2016, sprout wrote​:

On Tue Aug 16 15​:50​:31 2016, atoomic wrote​:

Recent change to perl 5.24 change the chown behavior when using -1
argument​:
f95ba54

But perldoc for chow still mention​:
"A value of -1 in either position is interpreted by most systems to
leave that value unchanged."

I'm not sure which part is incorrect​: the doc ? or the change ?
My code was currently using the chown -1 feature documented.

Is it now deprecated ?
Should we revert this commit or update the doc ?

Thanks for the clarification on this.
nicolas

# chown root​: /tmp/xxx; perl522 -E 'open my $f, ">", "/tmp/xxx";
say
chown -1, 99, $f'; ls -l /tmp/xxx
1
-rw-r--r-- 1 root nobody 0 Aug 16 16​:45 /tmp/xxx

# chown root​: /tmp/xxx; perl524 -E 'open my $f, ">", "/tmp/xxx";
say
chown -1, 99, $f'; ls -l /tmp/xxx
0
-rw-r--r-- 1 root root 0 Aug 16 16​:46 /tmp/xxx

Commit f95ba54 looks like a mistake to me. The change was made
to
appease a linter that didn’t like the idea of a negative argument
because someone decided at some point to make it not like it.

Makes no sense to me.

Looks like the negative arguments to fchown are supported in some
systems, with some semantics, under some conditions. In other words,
nothing portable can be said about it. So reverting the change and
telling Coverity to shut up about it.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 17, 2016

From @cpansprout

On Wed Aug 17 07​:47​:43 2016, atoomic wrote​:

Please also revert
c4f643b
at the same time you are reverting
f95ba54

Can Jarkko explain the purpose of c4f643b?

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 18, 2016

From @atoomic

looks like the second commit happened after the first one in order to fix the unit test as -1 was not available anymore for chown....
that's why I think they are going together, but maybe Jarkko was addressing another problem ?

On Wed Aug 17 16​:40​:40 2016, sprout wrote​:

On Wed Aug 17 07​:47​:43 2016, atoomic wrote​:

Please also revert
c4f643b
at the same time you are reverting
f95ba54

Can Jarkko explain the purpose of c4f643b?

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 15, 2016

From @tonycoz

On Wed, 17 Aug 2016 07​:47​:43 -0700, atoomic wrote​:

Please also revert
c4f643b
at the same time you are reverting
f95ba54

Done in 6225847.

Tony

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 29, 2016

From @jkeenan

On Tue, 15 Nov 2016 04​:13​:58 GMT, tonyc wrote​:

On Wed, 17 Aug 2016 07​:47​:43 -0700, atoomic wrote​:

Please also revert
c4f643b
at the same time you are reverting
f95ba54

Done in 6225847.

Tony

Are there any issues outstanding for this ticket?

Thank you very much.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 29, 2016

From @atoomic

we can close this ticket the revert was done, and now everything is in sync

On Thu, 29 Dec 2016 06​:11​:58 -0800, jkeenan wrote​:

On Tue, 15 Nov 2016 04​:13​:58 GMT, tonyc wrote​:

On Wed, 17 Aug 2016 07​:47​:43 -0700, atoomic wrote​:

Please also revert
c4f643b
at the same time you are reverting
f95ba54

Done in 6225847.

Tony

Are there any issues outstanding for this ticket?

Thank you very much.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 29, 2016

From @jkeenan

On Thu, 29 Dec 2016 18​:52​:08 GMT, atoomic wrote​:

we can close this ticket the revert was done, and now everything is in
sync

On Thu, 29 Dec 2016 06​:11​:58 -0800, jkeenan wrote​:

On Tue, 15 Nov 2016 04​:13​:58 GMT, tonyc wrote​:

On Wed, 17 Aug 2016 07​:47​:43 -0700, atoomic wrote​:

Please also revert
c4f643b
at the same time you are reverting
f95ba54

Done in 6225847.

Tony

Are there any issues outstanding for this ticket?

Thank you very much.

Marking ticket Resolved.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 29, 2016

@jkeenan - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Dec 29, 2016
@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.