pbc_disassemble dumps core on illegal options

[doughera]

In src/pbc_disassemble.c , The longopt_opt_decl structure isn't terminated. This can cause src/longopt.c to walk off the end of the array. This was uncovered by one of the recently added tests in t/tools/pbc_disassemble.t. The attached patch fixes it.

[doughera]

Terminate options array.

[jkeenan]

Patch applied:

901764e | jkeenan++ | / (2 files): Merge branch 'doughera_3patches'. Applies patches submitted by doughera++ in TT #1950 and TT #1953.

Andy, please re-test.

Thank you very much.

kid51

[doughera]

On Wed, 12 Jan 2011, Parrot wrote:

> #1950: pbc_disassemble dumps core on illegal options

>  Andy, please re-test.

My fault -- I think we'll need this follow-up too, to get the correct 
types in the structure for very strict compilers.  (I've fixed the 
documentation to match.)  Sorry no attachment here, but that's how I'm
connected at the moment:

diff --git a/docs/dev/longopt.pod b/docs/dev/longopt.pod
index a346587..c266e87 100644
--- a/docs/dev/longopt.pod
+++ b/docs/dev/longopt.pod
@@ -21,18 +21,18 @@ id (generally the same as the short option), some flags, and finally a list of
 up to nine long options (all for this one behavior), terminated with a NULL
 pointer.
 
-There is currently one possible flag: OPTION_required_FLAG, which states that
-this option has a required argument.  Optional arguments are not supported, and
-they should be.
+There are currently two possible flags: OPTION_required_FLAG, which states that
+this option has a required argument, and OPTION_optional_FLAG, which indicates 
+that this option has an optional argument.
 
-The array should be terminated with an element that has 0 for the option id.
+The array must be terminated with an element that has 0 for the option id.
 So, for example:
 
     struct longopt_opt_decl options[] = {
         { 'f', 'f', 0,                    { "--foo", NULL } },
         { 'b', 'b', OPTION_required_FLAG, { "--bar", NULL } },
         {   0, 128, 0,                    { "--baz", "--bazbar", NULL } },
-        {   0,   0, 0,                    { NULL } }
+        {   0,   0, OPTION_optional_FLAG, { NULL } }
     };
 
 This is a structure that specifies three options.
diff --git a/src/pbc_disassemble.c b/src/pbc_disassemble.c
index b6efbd8..88af455 100644
--- a/src/pbc_disassemble.c
+++ b/src/pbc_disassemble.c
@@ -76,7 +76,7 @@ static struct longopt_opt_decl options[] = {
     { 'D', 'D', OPTION_required_FLAG, { "--debug" } },
 #endif
     { 'o', 'o', OPTION_required_FLAG, { "--output" } },
-    {  0 ,  0,  0,                    { NULL } }
+    {  0 ,  0,  OPTION_optional_FLAG, { NULL } }
 };
 
 /*


-- 
    Andy Dougherty		doughera@lafayette.edu

[jkeenan]

I have applied this patch locally and so far it is passing all its testing targets on Linux/i386.

But, since we're close to 3.0, I will wait until I get a thumbs up from cotto before applying to master.

Thank you very much.

kid51

[coke]

IRC log for #parrot, 2011-01-13

05:19 cotto msg kid51 +1 to apply the 1950 patch to fix pbc_disassemble.

[doughera]

Replying to jkeenan:

I have applied this patch locally and so far it is passing all its testing targets on Linux/i386. But, since we're close to 3.0, I will wait until I get a thumbs up from cotto before applying to master.

The code fix is already in master and works. The matching documentation fix to docs/dev/longopt.pod should go in as well, then this ticket can be closed.

[jkeenan]

commit 2ca82d9: TT #1950: Applying updated patches from doughera++.

Closing ticket.

[cotto]

It looks like this was partially reverted in 59faf46dc to fix the C++ build. (The commit message doesn't mention it but dukeleto verified that that was the intent.) Can someone fix this bug in a way that preserves the C++ build?

[doughera]

Replying to cotto:

It looks like this was partially reverted in 59faf46dc to fix the C++ build. (The commit message doesn't mention it but dukeleto verified that that was the intent.) Can someone fix this bug in a way that preserves the C++ build?

It's fine as is. Reclosing ticket.