Re: Allow equal after a short option
On Mar 9, 2008, at 10:14 AM, Wayne Davison wrote:
On Sun, Mar 09, 2008 at 12:42:37AM -0500, Jeff Johnson wrote:
/bin/echo on my system is unmodified from
Fedora 9 coreutils-6.10-4.fc9.i386
Interesting. So, what do you get with a manual run?
/bin/echo --foo --bar
/bin/echo -- --foo --bar
[EMAIL PROTECTED] popt]$ /bin/echo --foo --bar
--bar
[EMAIL PROTECTED] popt]$ /bin/echo -- --foo --bar
--foo --bar
Heh, I believe the answer starts to become clearer:
The echo(1) from Fedora coreutils-6.10-4.fc9.i386 dares to be
different.
The root cause is what I needed to understand. Thanks for the patience.
I see all the option information output literally, including the --.
What do you get if you try a make check using that perl -e patch
instead of echo? Does it still succeed for you?
I added the longArg = NULL, am seeing the same failure on test # 9.
Very weird. I don't see how my change could affect a short option's
separated arg. Attached is an even safer version of the change that
ensures that the only time it ever sets longArg to NULL is if the
longArg was set to oe + 1 upon finding an equal sign.
I also tried using valgrind in the test suite:
result=`valgrind $builddir/$prog $*`
and the test-run didn't turn up any errors.
Your
longArg = NULL;
patch seems fine. The issue I'm seeing is here:
...
origOptString++;
if (*origOptString != '\0')
con-os-nextCharArg = origOptString;
#ifdef NOTYET /* XXX causes test 9 failure. */
con-os-nextCharArg = origOptString +
(*origOptString == '=');
#endif
}
if (opt == NULL) return POPT_ERROR_BADOPT; /* XXX can't
happen */
...
If I turn that line on, popt make check (and rpm -q) breaks.
There's a class of problems, particularly with recursions like popt,
that can fool valgrind.
Meanwhile, below is a rewrite of POPT_fprintf, essentially identical
to the man vsnprintf
example. See what you think ...
(aside) I swear I wrote this code before, likely early June 2007,
when the need for va_copy
on amd64/ppc was discovered. The issue was discussed at some length
on [EMAIL PROTECTED]
during the 1st two weeks of June 2007.
int
POPT_fprintf (FILE * stream, const char * format, ...)
{
char * b = NULL, * ob = NULL;
size_t nb = 1;
int rc;
va_list ap;
while ((b = realloc(b, nb)) != NULL) {
va_start(ap, format);
rc = vsnprintf(b, nb, format, ap);
va_end(ap);
if (rc -1 (size_t)rc nb)
break;
if (rc -1)/* glibc 2.1 */
nb = rc + 1;
else/* glibc 2.0 */
nb += (nb = 1 ? 100 : nb);
ob = b;
}
rc = 0;
if (b != NULL) {
#ifdef HAVE_ICONV
ob = strdup_locale_from_utf8(b);
if (ob != NULL) {
rc = fprintf(stream, %s, ob);
free(ob);
} else
#endif
rc = fprintf(stream, %s, b);
free (b);
} else if (ob != NULL)
free(ob);
return rc;
}
73 de Jeff
__
POPT Library http://rpm5.org
Developer Communication List popt-devel@rpm5.org
Re: Allow equal after a short option
On Sat, Mar 08, 2008 at 12:10:52PM -0500, Jeff Johnson wrote:
Running test test1 - 9.
Test test1 -2 foo failed with: arg1: 0 arg2: rest: foo != arg1:
0 arg2: foo
I can get that failure if the line I added does not replace the prior
assignment (which makes it affect the case where *origOptString == '\0'
as well as the desired case where it is not '\0').
That's the only explanation I can come up with for why the code would
fail. I have attached a patch that codes up the increment in a slightly
different way, but I don't see how this change is any different on the
code that follows than what was there before. (Still, I might have
missed something...)
..wayne..
--- popt.c 9 Mar 2008 20:24:45 - 1.119
+++ popt.c 9 Mar 2008 22:15:08 -
@@ -931,11 +931,11 @@ int poptGetNextOpt(poptContext con)
shorty = 1;
origOptString++;
- if (*origOptString != '\0')
+ if (*origOptString != '\0') {
+ if (*origOptString == '=')
+ origOptString++;
con-os-nextCharArg = origOptString;
-#ifdef NOTYET /* XXX causes test 9 failure. */
- con-os-nextCharArg = origOptString + (*origOptString == '=');
-#endif
+ }
}
if (opt == NULL) return POPT_ERROR_BADOPT; /* XXX can't happen */
Re: Allow equal after a short option
On Sun, Mar 09, 2008 at 07:10:21PM -0400, Jeff Johnson wrote: Your original patch for -c=foo is now checked in. Cool! You should also be able to uncomment the extra tests now. ..wayne.. __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Allow equal after a short option
On Mar 8, 2008, at 12:02 PM, Wayne Davison wrote: I think it would be nice to allow an equal to separate a short option letter from its abutting argument. e.g. these commands using the test1 executable would all work the same: ./test1 -2 foo ./test1 -2=foo ./test1 -2foo ./test1 --arg2 foo ./test1 --arg2=foo Since this has been a syntax error in released versions of popt, this should not cause a compatibility problem. This fix requires my prior patch to make sure that short-option parsing doesn't have longArg set. I think the patch is a reasonable extension, but there's more to do: [EMAIL PROTECTED] popt]$ make check ... make check-TESTS make[2]: Entering directory `/X/src/popt' Running tests in /X/src/popt Running test test1 - 1. Running test test1 - 2. Running test test1 - 3. Running test test1 - 4. Running test test1 - 5. Running test test1 - 6. Running test test1 - 7. Running test test1 - 8. Running test test1 - 9. Test test1 -2 foo failed with: arg1: 0 arg2: rest: foo != arg1: 0 arg2: foo Would you mind looking at the problem? Thanks ... 73 de Jeff __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Allow equal after a short option
On Sat, Mar 08, 2008 at 12:10:52PM -0500, Jeff Johnson wrote: Test test1 -2 foo failed with: arg1: 0 arg2: rest: foo != arg1: 0 arg2: foo I'm not seeing that error with the CVS version. I do note that my prior patch to fix the longArg pointer (e.g. ./test1 -2foo=bar) isn't there, but even commenting out that fix doesn't get test 9 to fail for me. I did see failures for tests 19-23 due to the --echo-args macro now outputting an extra -- at the start. The attached patch fixes this, and also adds two new tests to check that the changed equal-handling works right. ..wayne.. popt-tests.sh Description: Bourne shell script
Re: Allow equal after a short option
On Sat, Mar 08, 2008 at 06:11:09PM -0500, Jeff Johnson wrote:
Hmmm, we appear to have different behavior wrto echo. Your
patch changes testit.sh to include an explicit --, which (when
I last fixed testit.sh like 3 weeks ago) does not appear in the
output I am (and was) seeing.
I tried it on Ubuntu 7.10 and CentOS 5 with the same result, so it's
obviously a difference between whatever version of echo you have and
the one in the gnu coreutils package. I'll attach a patch that makes
the code use a simple perl -e construct to accomplish the same thing in
a compatible manner (for any system with perl). Using that avoids the
need to add the -- chars to the output like I did in my earlier patch.
I have added the tests and the 1 liner to have -c=foo functionality,
just commented out and disabled for now.
Please note that that one-line fix won't work without my prior patch
that fixes the problem with a short option that has an embedded (or
leading) equal in an abutting arg (e.g. test1 -2foo=bar). I'll attach
it here in case you missed it.
..wayne..
--- test-poptrc 16 Feb 2008 22:16:10 - 1.4
+++ test-poptrc 9 Mar 2008 04:13:55 -
@@ -7,6 +7,6 @@ test1 alias -O --arg1
test1 alias --grab --arg2 'foo !#:+'
test1 alias --grabbar --grab bar
-test1 exec --echo-args echo --
+test1 exec --echo-args perl -e 'print @ARGV' --
test1 alias -e --echo-args
-test1 exec -a /bin/echo --
+test1 exec -a perl -e 'print @ARGV' --
--- popt.c 8 Mar 2008 17:26:30 - 1.116
+++ popt.c 8 Mar 2008 20:48:43 -
@@ -901,6 +901,7 @@ int poptGetNextOpt(poptContext con)
if (!opt) {
con-os-nextCharArg = origOptString + 1;
+ longArg = NULL;
} else {
if (con-os == con-optionStack F_ISSET(opt, STRIP))
{
