Re: Question about the xmalloc() et al functions
On Sun, Mar 09, 2008 at 07:00:52PM -0700, Wayne Davison wrote:
> +} else if (prtshort) {
> + left[0] = '-';
> + left[1] = opt->shortName;
> +} else if (prtlong) {
Oops, there should have been a left[2] = '\0' in there.
..wayne..
__
POPT Library http://rpm5.org
Developer Communication List popt-devel@rpm5.org
Re: Question about the xmalloc() et al functions
On Sun, Mar 09, 2008 at 08:27:25PM -0400, Jeff Johnson wrote:
> Using xmalloc just opens up a can-of-worms while lusers fuss about
> non-gcc compiler extension portability.
Aha, I had failed to notice that the "? :" bit was a gcc extension.
What I don't like is that normal users get some memory functions that
are fatal if they use gcc, while all others get non-fatal functions.
Perhaps the default should be for all normal compiles to get the non-
fatal functions, and then anyone that wanted to use the current gcc
defines could set something extra (such as -DFATAL_MEM).
One other reason I care about this is that I think the various bits of
code that are doing a strdup()-like action should be using a strdup()-
like call, not each one having their own set of strlen/malloc/strcpy
calls. It's clearer and a bit safer.
I'm attaching a patch that adds the extra FATAL_MEM requirement, and
then uses xstrdup() in a few places (which would mean that strdup() is
being used for normal popt users).
I was also working on changing some code to use your newly-added
stpcpy() function, so I included that work too. Some of the changes
are optimizations to avoid a strlen() call (when we can easily compute
the length via subtraction) and a fix for a potential memory leak if
realloc() returns NULL.
..wayne..
--- system.h9 Mar 2008 20:24:45 - 1.14
+++ system.h10 Mar 2008 01:40:23 -
@@ -78,7 +78,7 @@ static inline char * stpcpy (char *dest,
#endif
/* Memory allocation via macro defs to get meaningful locations from mtrace()
*/
-#if defined(HAVE_MCHECK_H) && defined(__GNUC__)
+#if defined(FATAL_MEM) && defined(HAVE_MCHECK_H) && defined(__GNUC__)
#definevmefail() (fprintf(stderr, "virtual memory
exhausted.\n"), exit(EXIT_FAILURE), NULL)
#definexmalloc(_size) (malloc(_size) ? : vmefail())
#definexcalloc(_nmemb, _size) (calloc((_nmemb), (_size)) ? :
vmefail())
--- findme.c9 Mar 2008 20:24:45 - 1.19
+++ findme.c10 Mar 2008 01:40:21 -
@@ -23,11 +23,10 @@ const char * POPT_findProgramPath(const
if (path == NULL) return NULL;
-start = pathbuf = malloc(strlen(path) + 1);
+start = pathbuf = xstrdup(path);
if (pathbuf == NULL) goto exit;
buf = malloc(strlen(path) + strlen(argv0) + sizeof("/"));
if (buf == NULL) goto exit;
-strcpy(pathbuf, path);
chptr = NULL;
do {
--- popt.c 9 Mar 2008 23:10:05 - 1.120
+++ popt.c 10 Mar 2008 01:40:22 -
@@ -196,10 +196,8 @@ poptContext poptGetContext(const char *
if (getenv("POSIXLY_CORRECT") || getenv("POSIX_ME_HARDER"))
con->flags |= POPT_CONTEXT_POSIXMEHARDER;
-if (name) {
- char * t = malloc(strlen(name) + 1);
- if (t) con->appName = strcpy(t, name);
-}
+if (name)
+ con->appName = xstrdup(name);
invokeCallbacksPRE(con, con->options);
@@ -593,7 +591,7 @@ expandNextArg(/[EMAIL PROTECTED]@*/ poptContext
/[EMAIL PROTECTED] con @*/
{
const char * a = NULL;
-size_t alen;
+size_t alen, pos;
char *t, *te;
size_t tn = strlen(s) + 1;
char c;
@@ -619,10 +617,9 @@ expandNextArg(/[EMAIL PROTECTED]@*/ poptContext
alen = strlen(a);
tn += alen;
- *te = '\0';
+ pos = te - t;
t = realloc(t, tn);
- te = t + strlen(t);
- strncpy(te, a, alen); te += alen;
+ te = stpcpy(t + pos, a);
continue;
/[EMAIL PROTECTED]@*/ /[EMAIL PROTECTED]@*/ break;
default:
@@ -630,8 +627,13 @@ expandNextArg(/[EMAIL PROTECTED]@*/ poptContext
}
*te++ = c;
}
-*te = '\0';
-t = realloc(t, strlen(t) + 1); /* XXX memory leak, hard to plug */
+*te++ = '\0';
+if (t + tn > te) {
+ char *ot = t;
+ t = realloc(t, te - t);
+ if (!t)
+ free(ot);
+}
return t;
}
--- poptconfig.c9 Mar 2008 20:24:45 - 1.37
+++ poptconfig.c10 Mar 2008 01:40:22 -
@@ -224,8 +224,7 @@ int poptReadDefaultConfig(poptContext co
if ((home = getenv("HOME"))) {
fn = malloc(strlen(home) + 20);
if (fn != NULL) {
- strcpy(fn, home);
- strcat(fn, "/.popt");
+ (void)stpcpy(stpcpy(fn, home), "/.popt");
rc = poptReadConfigFile(con, fn);
free(fn);
} else
--- popthelp.c 8 Mar 2008 17:26:30 - 1.85
+++ popthelp.c 10 Mar 2008 01:40:23 -
@@ -237,7 +237,7 @@ singleOptionDefaultValue(size_t lineLeng
if (le == NULL) return NULL; /* XXX can't happen */
*le = '\0';
*le++ = '(';
-strcpy(le, defstr);le += strlen(le);
+le = stpcpy(le, defstr);
*le++ = ':';
*le++ = ' ';
if (opt->arg) { /* XXX programmer error */
@@ -269,7 +269,7 @@ singleOptionDefaultValue(size_t lineLeng
case POPT_ARG_STRING:
{ const char * s = arg.argv[0];
if (s == NULL) {
- s
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: Question about the xmalloc() et al functions
On Mar 9, 2008, at 7:59 PM, Wayne Davison wrote: On Sun, Mar 09, 2008 at 07:26:31PM -0400, Jeff Johnson wrote: There are some subtleties however with xstrdup. [... mtrace related issues ...] Right, that's why I preserved the HAVE_MCHECK_H version using its own malloc() call (it's unchanged from your version) and just made the non- HAVE_MCHECK_H version add failure checking. All the other functions are the same for both (with exit-on-failure handling). I'd love to use xmalloc everywhere. Alas, there's still lots of dain bread portability issues, the patches to "fix" portability are usually incorrect and limited in scope. See e.g. __secure_getenv and the setresgid/ setresuid "stuff", to name just two. The va_copy() hackery entered popt through the same Newer! Better! Bestest! window, where popt is now undertaking iconv conversions and bind_textdomain_codeset() retrofits that _REALLY_ need to be solved in applications, not in popt. Using xmalloc just opens up a can-of-worms while lusers fuss about non-gcc compiler extension portability. Again, I'd *love* to use xmalloc and xstrdup, I use daily, worksforme. But at some point, popt should just process options, no fuss, no muss, no problems. 73 de Jeff __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Question about the xmalloc() et al functions
On Sun, Mar 09, 2008 at 07:26:31PM -0400, Jeff Johnson wrote: > There are some subtleties however with xstrdup. [... mtrace related > issues ...] Right, that's why I preserved the HAVE_MCHECK_H version using its own malloc() call (it's unchanged from your version) and just made the non- HAVE_MCHECK_H version add failure checking. All the other functions are the same for both (with exit-on-failure handling). ..wayne.. __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Question about the xmalloc() et al functions
On Mar 9, 2008, at 7:13 PM, Wayne Davison wrote: I'm curious why the xmalloc() function (and its brethren) exits with a fatal error on only some systems and not on all systems? I would think that the code should use the exiting functions in all cases, and just provide differing versions of xstrdup() so that the HAVE_MCHECK_H version does a malloc() (rather than strdup() doing it). xmalloc() et al is mostly a fetish of mine. I personally believe that its silly wasting time checking _ALL_ code paths for malloc failures. 99.999% of the time returning an error rather than aborting is pointless anyways (imho). If I'm not off track, you can apply the attached patch to implement this. There are some subtleties however with xstrdup. If strdup(3), rather than what I use with rpm #define xstrdup(_str) (strcpy((malloc(strlen(_str)+1) ? : vmefail(strlen(_str)+1)), (_str))) is done, then line numbers reported by glibc mtrace(3) will be wrto strdup, not the caller of strdup. All that stops me from using everywhere is the GNU extension that is needed by xstrdup so that the xstrdup macro is mostly side-effect free (I don't care so much that the (_str) macro argument is used several times on the error paths, its used only once on the success paths. And sure a static inline function avoids all the side-effects, but static inline functions were unknown to me last century when the hackery was first devised. valgrind dinna exist, and Rational Purify cost the Very Big $$$ back then too ... 73 de Jeff __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Question about the xmalloc() et al functions
I'm curious why the xmalloc() function (and its brethren) exits with a fatal error on only some systems and not on all systems? I would think that the code should use the exiting functions in all cases, and just provide differing versions of xstrdup() so that the HAVE_MCHECK_H version does a malloc() (rather than strdup() doing it). If I'm not off track, you can apply the attached patch to implement this. ..wayne.. --- system.h9 Mar 2008 20:24:45 - 1.14 +++ system.h9 Mar 2008 23:01:40 - @@ -77,19 +77,17 @@ static inline char * stpcpy (char *dest, } #endif -/* Memory allocation via macro defs to get meaningful locations from mtrace() */ -#if defined(HAVE_MCHECK_H) && defined(__GNUC__) +/* Error-checking versions of common memory-allocation functions. */ #definevmefail() (fprintf(stderr, "virtual memory exhausted.\n"), exit(EXIT_FAILURE), NULL) #definexmalloc(_size) (malloc(_size) ? : vmefail()) #definexcalloc(_nmemb, _size) (calloc((_nmemb), (_size)) ? : vmefail()) #definexrealloc(_ptr, _size) (realloc((_ptr), (_size)) ? : vmefail()) +/* Avoid strdup() to get meaningful locations from mtrace() */ +#if defined(HAVE_MCHECK_H) && defined(__GNUC__) #define xstrdup(_str) (strcpy((malloc(strlen(_str)+1) ? : vmefail()), (_str))) #else -#definexmalloc(_size) malloc(_size) -#definexcalloc(_nmemb, _size) calloc((_nmemb), (_size)) -#definexrealloc(_ptr, _size) realloc((_ptr), (_size)) -#definexstrdup(_str) strdup(_str) -#endif /* defined(HAVE_MCHECK_H) && defined(__GNUC__) */ +#define xstrdup(_str) (strdup(_str) ? : vmefail()) +#endif #if defined(HAVE___SECURE_GETENV) && !defined(__LCLINT__) #definegetenv(_s) __secure_getenv(_s)
Re: Allow equal after a short option
On Mar 9, 2008, at 6:36 PM, Wayne Davison wrote:
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...)
Bingo.
My brain fart, nothing more. Tired old eyes again again, sigh.
Note the missing { ... }, I applied your original patch incorrectly.
Here's the fix:
Index: popt.c
===
RCS file: /v/rpm/cvs/popt/popt.c,v
retrieving revision 1.119
diff -u -b -B -w -p -r1.119 popt.c
--- popt.c 9 Mar 2008 20:24:45 - 1.119
+++ popt.c 9 Mar 2008 23:05:31 -
@@ -932,8 +932,7 @@ int poptGetNextOpt(poptContext con)
origOptString++;
if (*origOptString != '\0')
- con->os->nextCharArg = origOptString;
-#ifdef NOTYET /* XXX causes test 9 failure. */
+#ifndefNOTYET /* XXX causes test 9 failure. */
con->os->nextCharArg = origOptString +
(*origOptString == '=');
#endif
}
Your original patch for -c=foo is now checked in.
73 de Jeff
__
POPT Library http://rpm5.org
Developer Communication List popt-devel@rpm5.org
Re: echo incompatibility (was Re: Allow equal after a short option)
On Mar 9, 2008, at 6:22 PM, Wayne Davison wrote: On Sun, Mar 09, 2008 at 10:51:42AM -0400, Jeff Johnson wrote: [EMAIL PROTECTED] popt]$ /bin/echo --foo --bar --bar [EMAIL PROTECTED] popt]$ /bin/echo -- --foo --bar --foo --bar OK, Let's hope that your echo will not drop anything if the first arg passed to it is a non-option (such as "args:"). If so, the attached patch should work for everyone (it does for me). Clever hack to change the 1st arg to echo ... However, not-POSIXLY_CORRECT rearrangements _SHOULD_ cause argument permutation with, say, /bin/echo args: A -b C -d --foo --bar afaict that may exhibit the same failure lossage as before. Dunno, non-POSIXLY_CORRECT echo(1) argument processing definition(s) I'll leave to LSB-53, "unspecified" behavior is gud enuf for me. The better fix (for me as well as popt) is to get a non-Fedora enhanced /bin/echo ... todo++. 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 */
echo incompatibility (was Re: Allow equal after a short option)
On Sun, Mar 09, 2008 at 10:51:42AM -0400, Jeff Johnson wrote: > [EMAIL PROTECTED] popt]$ /bin/echo --foo --bar > --bar > [EMAIL PROTECTED] popt]$ /bin/echo -- --foo --bar > --foo --bar OK, Let's hope that your echo will not drop anything if the first arg passed to it is a non-option (such as "args:"). If so, the attached patch should work for everyone (it does for me). ..wayne.. --- test-poptrc 16 Feb 2008 22:16:10 - 1.4 +++ test-poptrc 9 Mar 2008 22:15:08 - @@ -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 echo args: test1 alias -e --echo-args -test1 exec -a /bin/echo -- +test1 exec -a /bin/echo args: --- testit.sh 8 Mar 2008 23:11:56 - 1.24 +++ testit.sh 9 Mar 2008 22:15:08 - @@ -68,11 +68,11 @@ POSIXLY_CORRECT=1 ; export POSIXLY_CORRE run test1 "test1 - 17" "arg1: 1 arg2: (none) rest: foo --arg2 something" --arg1 foo --arg2 something unset POSIXLY_CORRECT run test1 "test1 - 18" "callback: c sampledata bar arg1: 1 arg2: (none)" --arg1 --cb bar -run test1 "test1 - 19" "" --echo-args -run test1 "test1 - 20" "--arg1" --echo-args --arg1 -run test1 "test1 - 21" "--arg2 something" -T something -e -run test1 "test1 - 22" "--arg2 something more args" -T something -a more args -run test1 "test1 - 23" "--echo-args -a" --echo-args -e -a +run test1 "test1 - 19" "args:" --echo-args +run test1 "test1 - 20" "args: --arg1" --echo-args --arg1 +run test1 "test1 - 21" "args: --arg2 something" -T something -e +run test1 "test1 - 22" "args: --arg2 something more args" -T something -a more args +run test1 "test1 - 23" "args: --echo-args -a" --echo-args -e -a run test1 "test1 - 24" "arg1: 0 arg2: (none) short: 1" -onedash run test1 "test1 - 25" "arg1: 0 arg2: (none) short: 1" --onedash run test1 "test1 - 26" "callback: c arg for cb2 foo arg1: 0 arg2: (none)" --cb2 foo
Re: Allow equal after a short option
On Mar 9, 2008, at 5:38 PM, Wayne Davison wrote: On Sun, Mar 09, 2008 at 10:51:42AM -0400, Jeff Johnson wrote: Meanwhile, below is a rewrite of POPT_fprintf, essentially identical to the "man vsnprintf" example. See what you think ... Looks good. I did some tweaking based on this code. The attached patch changes what you checked-in to CVS. See if you agree with these points: 1. If the system has vasprintf(), just use that (avoiding issues 2 & 3). Agreed. 2. Always allocate one more byte than we need, just in case the system's vsprintf() has an off-by-one bug with the limit. That's kinda nutty (but hurts nothing). I'd rather fix a known problem rather than trying to anticipate all possibilities ... 3. Moved the va_start() and va_end() calls back in the loop because I believe that the systems that needed a va_copy() in the old code won't like the reuse of "ap" without it. ... afaik 3) is vs implementation dependent confusion with va_start() and va_end(). But hurts nothing at all ... Bonus question: I didn't think older systems supported the idiom of realloc(NULL, len) doing a malloc(). I didn't check to see if that idiom is already used elsewhere in the popt code, though, so I just left that alone. ... and my ultimate answer is: Deranged and ancient uglix-like systems, with busted vsnpprintf and other damage, can certainly live with gud old fprintf(3) as popt _USED_ to do without all this dain bread conversion from UTF-* to the native locale using iconv & vsnprintf & nl_langinfo & wide characters & malloc'ing fprintf's & None of this modern crapola existed on systems that had busted v*printf() implementations, almost all the problems disappear with #define POPT_fprintffprintf (and some tweaks to test whether POPT_fprintf is defined). KISS is always better ... I tested both sides of the HAVE_VASPRINTF code and this seems good to me. Off to tweak in vasprintf(3) ... I am happy to see sprintf start to vanish ... 73 de Jeff __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Allow equal after a short option
On Sun, Mar 09, 2008 at 10:51:42AM -0400, Jeff Johnson wrote:
> Meanwhile, below is a rewrite of POPT_fprintf, essentially identical
> to the "man vsnprintf" example. See what you think ...
Looks good. I did some tweaking based on this code. The attached patch
changes what you checked-in to CVS. See if you agree with these points:
1. If the system has vasprintf(), just use that (avoiding issues 2 & 3).
2. Always allocate one more byte than we need, just in case the system's
vsprintf() has an off-by-one bug with the limit.
3. Moved the va_start() and va_end() calls back in the loop because I
believe that the systems that needed a va_copy() in the old code
won't like the reuse of "ap" without it.
Bonus question: I didn't think older systems supported the idiom of
realloc(NULL, len) doing a malloc(). I didn't check to see if that
idiom is already used elsewhere in the popt code, though, so I just
left that alone.
I tested both sides of the HAVE_VASPRINTF code and this seems good to
me.
..wayne..
--- configure.ac9 Mar 2008 20:24:45 - 1.39
+++ configure.ac9 Mar 2008 21:26:17 -
@@ -69,7 +69,7 @@ AM_CONDITIONAL(HAVE_LD_VERSION_SCRIPT, t
AC_CHECK_FUNC(setreuid, [], [
AC_CHECK_LIB(ucb, setreuid, [if echo $LIBS | grep -- -lucb >/dev/null
;then :; else LIBS="$LIBS -lc -lucb" USEUCB=y;fi])
])
-AC_CHECK_FUNCS(getuid geteuid iconv mtrace __secure_getenv setregid stpcpy
strerror)
+AC_CHECK_FUNCS(getuid geteuid iconv mtrace __secure_getenv setregid stpcpy
strerror vasprintf)
AM_GNU_GETTEXT([external])
--- poptint.c 9 Mar 2008 20:24:45 - 1.17
+++ poptint.c 9 Mar 2008 21:26:18 -
@@ -152,13 +152,17 @@ int
POPT_fprintf (FILE * stream, const char * format, ...)
{
char * b = NULL, * ob = NULL;
-size_t nb = (size_t)1;
int rc;
va_list ap;
+#ifndef HAVE_VASPRINTF
+size_t nb = (size_t)1;
-va_start(ap, format);
-while ((b = realloc(b, nb)) != NULL) {
+/* Note that we always allocate 1 byte more than we need, just in
+ * case the vsnprintf() has an off-by-one bug with the limit. */
+while ((b = realloc(b, nb+1)) != NULL) {
+ va_start(ap, format);
rc = vsnprintf(b, nb, format, ap);
+ va_end(ap);
if (rc > -1) { /* glibc 2.1 */
if ((size_t)rc < nb)
break;
@@ -167,8 +171,16 @@ POPT_fprintf (FILE * stream, const char
nb += (nb < (size_t)100 ? (size_t)100 : nb);
ob = b;
}
+
+#else /* HAVE_VASPRINTF */
+
+va_start(ap, format);
+if (vasprintf(&b, format, ap) < 0)
+ b = NULL;
va_end(ap);
+#endif
+
rc = 0;
if (b != NULL) {
#ifdef HAVE_ICONV
RE: Allow equal after a short option
> [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. That is not cool (swallowing the first long option), especially since the echo built in to bash continues to work as it has. I also observe this behavior on F8. On the other hand, not emitting the -- seems reasonable, since it generically has the meaning "this signals the end of the options". (And no, that's not how it works anywhere else, including bash and ksh's echo on F8). __ POPT Library http://rpm5.org Developer Communication List popt-devel@rpm5.org
Re: Allow equal after a short option
On Mar 9, 2008, at 10:51 AM, Jeff Johnson wrote:
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)
This should have been
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;
}
73 de Jeff
__
POPT Library http://rpm5.org
Developer Communication List popt-devel@rpm5.org
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 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
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.
..wayne..
--- popt.c 9 Mar 2008 06:01:05 - 1.118
+++ popt.c 9 Mar 2008 14:03:00 -
@@ -883,6 +883,8 @@ int poptGetNextOpt(poptContext con)
optStringLen = (size_t)(oe - optString);
if (*oe == '=')
longArg = oe + 1;
+ else
+ oe = NULL;
/* XXX aliases with arg substitution need "--alias=arg" */
if (handleAlias(con, optString, optStringLen, '\0', longArg)) {
@@ -895,13 +897,16 @@ int poptGetNextOpt(poptContext con)
opt = findOption(con->options, optString, optStringLen, '\0',
&cb, &cbData,
singleDash);
- if (!opt && !singleDash)
- return POPT_ERROR_BADOPT;
+ if (!opt) {
+ if (!singleDash)
+ return POPT_ERROR_BADOPT;
+ if (oe)
+ longArg = NULL;
+ }
}
if (!opt) {
con->os->nextCharArg = origOptString + 1;
- longArg = NULL;
} else {
if (con->os == con->optionStack && F_ISSET(opt, STRIP))
{
