BUG: getopt does not permute argv correctly

[Eric Blake]

Hi!
I'm using cygwin64 with newlib 3.0.0. POSIXLY_CORRECT is not set.

Note that Cygwin provides its own getopt() implementation, rather than
relying on newlib. So you may be reporting to the wrong list.

$ ./test 1 2 3 -a -b -c
Newlib v3.0.0
POSIXLY_CORRECT=0x0
0: "./test"
1: "1"
2: "2"
3: "3"
4: "-a"
5: "-b"
6: "-c"
0: "./test"
1: "1"
2: "2"
3: "3"
4: "-a"
5: "-b"
6: "-c"
a=0, b=0, c=0

That behavior is POSIX compliant. Just because POSIXLY_CORRECT is not
set does not mean that you can expect non-POSIX behavior.

===> Failure!
I would expect that argv is permuted, so that -a -b -c come first. Also, I
expect a=1, b=1, c=1.

Your expectations do not match with POSIX. But they do match with Linux
behavior. Here's where cygwin does that:

winsup/cygwin/libc/getopt.c:
int
getopt(int nargc, char * const *nargv, const char *options)
{

/*
* We don't pass FLAG_PERMUTE to getopt_internal() since
* the BSD getopt(3) (unlike GNU) has never done this.
*
* Furthermore, since many privileged programs call getopt()
* before dropping privileges it makes sense to keep things
* as simple (and bug-free) as possible.
*/
return (getopt_internal(nargc, nargv, options, NULL, NULL, 0));

You may have a point that Cygwin should be patched to more closely match
Linux. Or you could use getopt_long() which permutes by default unless
POSIXLY_CORRECT is set. But for now, your observation matches the
Cygwin code, and is independent of newlib.

I also tried this program on an embedded ARM Cortex-M4 target, using the

test 1 2 3 -a -b -c

Newlib v2.5.0
POSIXLY_CORRECT=0x0
0: "test"
1: "1"
2: "2"
3: "3"
4: "-a"
5: "-b"
6: "-c"
0: "test"
1: "-a"
2: "-b"
3: "-c"
4: "1"
5: "2"
6: "3"
a=1, b=1, c=1
===> (somewhat surprisingly) OK!

That's because newlib's getopt() is different from Cygwin's, and DOES
permute by default. (Maybe Cygwin should be patched to just use
newlib's version, rather than duplicating things?)

test 1 2 3 -abc

Newlib v2.5.0
POSIXLY_CORRECT=0x0
0: "test"
1: "1"
2: "2"
3: "3"
4: "-abc"
0: "test"
1: "1"
2: "2"
3: "3"
4: "-abc"
a=1, b=1, c=1
===> Failure! a, b, c is parsed, but does not get permuted to argv[1].

Ouch - that looks like a bug in newlib. Patches are welcome!

[Joel Sherrill]

Thanks for your quick answer!
The problem is, that permute_from and num_nonopts is forgotten between
calls, if there is more than one option flag in an argv entry.
As a solution, I made those variables global, and added them to
getopt_data (similar to the existing optwhere).
Is there a place or framework for unit tests in newlib?
In the long run, I think it would be best to have only one common getopt
in newlib and cygwin. I suspect that very few embedded projects use
getopt, and so bugs like this could slip through for some years.

RTEMS uses getopt_r with our shell. Since we are a single process, each
commands "main" needs its own view of the area and argv processing.

Cygwin's getopt is used and tested much more often, and is about the same
in code complexity. Code size reductions, like using fputs() instead of
fprintf(), smaller permute(), and reentrant state could be ported
relatively easily.

Not disagreeing with you just pointing out that getopt() is only useful in
a single process embedded system if you only process arguments once.

--joel

Post by Eric Blake

Hi!
I'm using cygwin64 with newlib 3.0.0. POSIXLY_CORRECT is not set.

Note that Cygwin provides its own getopt() implementation, rather than
relying on newlib. So you may be reporting to the wrong list.

$ ./test 1 2 3 -a -b -c
Newlib v3.0.0
POSIXLY_CORRECT=0x0
0: "./test"
1: "1"
2: "2"
3: "3"
4: "-a"
5: "-b"
6: "-c"
0: "./test"
1: "1"
2: "2"
3: "3"
4: "-a"
5: "-b"
6: "-c"
a=0, b=0, c=0

That behavior is POSIX compliant. Just because POSIXLY_CORRECT is not
set does not mean that you can expect non-POSIX behavior.

===> Failure!
I would expect that argv is permuted, so that -a -b -c come first.
Also, I
expect a=1, b=1, c=1.

Your expectations do not match with POSIX. But they do match with Linux
int getopt(int nargc, char * const *nargv, const char *options) {
/*
* We don't pass FLAG_PERMUTE to getopt_internal() since
* the BSD getopt(3) (unlike GNU) has never done this.
*
* Furthermore, since many privileged programs call getopt()
* before dropping privileges it makes sense to keep things
* as simple (and bug-free) as possible.
*/
return (getopt_internal(nargc, nargv, options, NULL, NULL, 0));
You may have a point that Cygwin should be patched to more closely match
Linux. Or you could use getopt_long() which permutes by default unless
POSIXLY_CORRECT is set. But for now, your observation matches the
Cygwin code, and is independent of newlib.

I also tried this program on an embedded ARM Cortex-M4 target, using

test 1 2 3 -a -b -c

Newlib v2.5.0
POSIXLY_CORRECT=0x0
0: "test"
1: "1"
2: "2"
3: "3"
4: "-a"
5: "-b"
6: "-c"
0: "test"
1: "-a"
2: "-b"
3: "-c"
4: "1"
5: "2"
6: "3"
a=1, b=1, c=1
===> (somewhat surprisingly) OK!

That's because newlib's getopt() is different from Cygwin's, and DOES
permute by default. (Maybe Cygwin should be patched to just use newlib's
version, rather than duplicating things?)

test 1 2 3 -abc

Newlib v2.5.0
POSIXLY_CORRECT=0x0
0: "test"
1: "1"
2: "2"
3: "3"
4: "-abc"
0: "test"
1: "1"
2: "2"
3: "3"
4: "-abc"
a=1, b=1, c=1
===> Failure! a, b, c is parsed, but does not get permuted to argv[1].

Ouch - that looks like a bug in newlib. Patches are welcome!
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. [phone]
Virtualization: qemu.org | libvirt.org

--

[Corinna Vinschen]

Post by Joel Sherrill

Thanks for your quick answer!
The problem is, that permute_from and num_nonopts is forgotten between
calls, if there is more than one option flag in an argv entry.
As a solution, I made those variables global, and added them to
getopt_data (similar to the existing optwhere).
Is there a place or framework for unit tests in newlib?
In the long run, I think it would be best to have only one common getopt
in newlib and cygwin. I suspect that very few embedded projects use
getopt, and so bugs like this could slip through for some years.

RTEMS uses getopt_r with our shell. Since we are a single process, each
commands "main" needs its own view of the area and argv processing.

Keeping the code reentrant is a requirement.

Post by Joel Sherrill

Cygwin's getopt is used and tested much more often, and is about the same
in code complexity. Code size reductions, like using fputs() instead of
fprintf(), smaller permute(), and reentrant state could be ported
relatively easily.

Not disagreeing with you just pointing out that getopt() is only useful in
a single process embedded system if you only process arguments once.

If somebody is really interested to create a unified getopt source for
newlib and Cygwin, it should be a restart from scratch, using the latest
BSD sources. Even if Cygwin's version is newer and, presumably, better
tested, it's still pretty old code. Pulling in reentrancy and the few
Cygwin-specific changes shouldn't be too hard.


Corinna

[Corinna Vinschen]

Thanks for your quick answer!
The problem is, that permute_from and num_nonopts is forgotten between
calls, if there is more than one option flag in an argv entry.
As a solution, I made those variables global, and added them to
getopt_data (similar to the existing optwhere).

I'm not opposed to add this patch to newlib, but, did anybody
with a non-Cygwin system test it?


Thanks,
Corinna

[Corinna Vinschen]

Post by Corinna Vinschen

Thanks for your quick answer!
The problem is, that permute_from and num_nonopts is forgotten between
calls, if there is more than one option flag in an argv entry.
As a solution, I made those variables global, and added them to
getopt_data (similar to the existing optwhere).

I'm not opposed to add this patch to newlib, but, did anybody
with a non-Cygwin system test it?

From visual inspection the code looks reasonable, btw.


Corinna

[Corinna Vinschen]

Post by Corinna Vinschen

Post by Corinna Vinschen

Thanks for your quick answer!
The problem is, that permute_from and num_nonopts is forgotten between
calls, if there is more than one option flag in an argv entry.
As a solution, I made those variables global, and added them to
getopt_data (similar to the existing optwhere).

I'm not opposed to add this patch to newlib, but, did anybody
with a non-Cygwin system test it?

From visual inspection the code looks reasonable, btw.

Pushed.


Thanks,
Corinna