On Sun, Sep 4, 2011 at 12:59 PM, Bryan Kadzban
<br...@kadzban.is-a-geek.net> wrote:
> Bruce Dubbs wrote:
>> Doing some debugging, I'm making some progress.  I made some debug
>> printouts and in the first case got:
>>
>>    required_pkgconfig_version 999.999
>>
>> but in the second
>>
>>    required_pkgconfig_version a=b
>>
>> (which for some reason returns a positive number for
>> compare_versions ("0.26", "a=b") but that's a nonsense comparison due to
>> the invalid command line parsing.
>
> rpmvercmp() is screwy (in pkg.c).  It compares the two as numbers (since
> the first char of the first string is a digit), but ends up doing it
> piecewise: comparing "0" to "" (since the first char of the second arg
> is not a digit).  This triggers a line preceded with a comment pointing
> at RH bugzilla, bug 50977 -- the code returns 1 if the first item was
> numeric, and -1 otherwise.
>
> But the real bug is below.
>
>> I'm continuing to check.
>
> This looked interesting, so I started digging as well.  I think I found it.
>
> poptGetNextOption, after it invokes the callback for
> --define-variable=a=b (the operation of which sets the longArg local
> variable), does not reset longArg to NULL.  So on the next time through
> its loop (after invoking the --define-variable callback, before invoking
> the --atleast-pkgconfig-version callback), on line 389 of popt.c, sets
> con->os->nextArg to longArg (the old "a=b" string).  What it should do
> is set nextArg to nextCharArg (the next item on the command line).  This
> happens if longArg is NULL.
>
> Adding a "longArg = NULL;" as the first line of the "while (!done) {"
> loop makes the tests pass.  I don't *think* it breaks anything else, but
> I don't know for sure.
>
> Short patch attached.  It can probably be turned into a sed.
>
> diff -Naur pkg-config-0.26/popt/popt.c pkg-config-0.26-patched/popt/popt.c
> --- pkg-config-0.26/popt/popt.c 2011-05-15 02:06:06.000000000 -0700
> +++ pkg-config-0.26-patched/popt/popt.c 2011-09-04 12:44:26.000000000 -0700
> @@ -297,6 +297,8 @@
>     int singleDash;
>
>     while (!done) {
> +       longArg = NULL;
> +
>        while (!con->os->nextCharArg && con->os->next == con->os->argc
>                && con->os > con->optionStack)
>            con->os--;
>

Nice! Someone reported this issue upstream, which is why I added the
test. I tried looking through current popt for a fix, but it's
diverged quite a bit. Can you make this a real patch with description?
Sooner or later popt will be removed completely and we'll just use
glib instead, but it hasn't happened yet. I think this is the patch
for that:

http://cgit.freedesktop.org/~dbn/pkg-config/commit/?h=glib2&id=503670a7390f436bc173e68e9d97e1aadee5c679

--
Dan
-- 
http://linuxfromscratch.org/mailman/listinfo/lfs-dev
FAQ: http://www.linuxfromscratch.org/faq/
Unsubscribe: See the above information page

Reply via email to