[Monotone-devel] popt buggy

[Richard Levitte - VMS Whacker]

Hi,

after all the problems that appeared with -@, with all kind of garbage
being thrown in your face, memory errors and stuff, and discussions on
IRC, I took a good look at popt (Debian, 1.7-5).  Lo and behold, I
found a nasty bug!

The trouble happens when having options after non-options.  Popt will
simply save an array of pointers to those so called "leftovers".  And
that's actually fine...  except that if you have added on extra
arguments with poptStuffArgs(), the passed array (or rather, a
duplicate) is free'd after it's been completely parsed.  If there were
any non-option in there, the array of "leftovers" will suddenly have
dangling pointers, into an area where anything can happen.

So, for anyone who wants to venture into popt source, here follows a
patch I created to fix this particular problem, and that I'm going to
send back to the Debian maintainers.  It doesn't solve everything, but
it does remove the worst stumbling block.  With that, monotone can
actually handle several -@ on the command line with no problem.

# 
# patch "popt.c"
#  from [da9dc1fd6caaceac5c156930a9540ce7bca0ed89]
#    to [0423e3055a4962c8bc2c10c471f87507e920d20a]
# 
--- popt.c
+++ popt.c
@@ -732,7 +732,10 @@
                    return 0;
                }
                if (con->leftovers != NULL)     /* XXX can't happen */
-                   con->leftovers[con->numLeftovers++] = origOptString;
+                   con->leftovers[con->numLeftovers++]
+                       = xstrdup(origOptString); /* so a free of a stuffed
+                                                    argv doesn't give us a
+                                                    dangling pointer */
                continue;
            }
 
@@ -1066,7 +1069,11 @@
     }
     con->execs = _free(con->execs);
 
+    for (i = 0; i < con->numLeftovers; i++) {
+       _free(con->leftovers + i);
+    }
     con->leftovers = _free(con->leftovers);
+
     con->finalArgv = _free(con->finalArgv);
     con->appName = _free(con->appName);
     con->otherHelp = _free(con->otherHelp);

-----
Please consider sponsoring my work on free software.
See http://www.free.lp.se/sponsoring.html for details.

-- 
Richard Levitte                         address@hidden
                                        http://richard.levitte.org/

"When I became a man I put away childish things, including
 the fear of childishness and the desire to be very grown up."
                                                -- C.S. Lewis