Bug in Stellaris Firmware Development package - "utils/cmdline.c"

Adam Brailove

It looks like a S/W bug to me in utils/cmdline.c Can someone at TI have a look?

I am getting faults due to memory dereferences beyond the end of the command table.

See code line approx 152.

When the command is not found in the command table it increments pCmdEntry past the end of the table.

Then the argument to the next test of the while loop dereferences that pointer . The argument of the while() is a POINTER to a char and is NOT NULL.

On the other hand the inline comments suggent the table is terminated by a NULL command string. This would require a dereference in the while argument and an additional null entry into the table which is not shown in the exaple code.

What was the intent: null string or null pointer?

Thanks

-A

// Search through the command table until a null command string is

// found, which marks the end of the table.

while(pCmdEntry->pcCmd)

---------------------------

if(argc)

//

// Start at the beginning of the command table, to look for a matching

// command.

//

        pCmdEntry = &g_sCmdTable[0];

//

// Search through the command table until a null command string is

// found, which marks the end of the table.

//

while(pCmdEntry->pcCmd)

//

// If this command entry command string matches argv[0], then call

// the function for this command, passing the command line

// arguments.

//

if(!strcmp(argv[0], pCmdEntry->pcCmd))

return(pCmdEntry->pfnCmd(argc, argv));

//

// Not found, so advance to the next entry.

//

            pCmdEntry++;

over 12 years ago

0 Dave Wilson over 12 years ago

TI__Mastermind 26900 points

I think this is a case where some more explicit information is needed in the function documentation. The intent (as you will see from all the working examples containing this module) is that the command table be terminated with a structure whose pcCmd field contains a NULL pointer. I tend to think of NULL string and NULL pointer as the same thing but I expect a NULL string could be a valid pointer to a buffer whose first byte is a 0.

I've updated the documentation on both the structure and the CmdLineProcess() function to make this clear.

0 Andy Neil over 12 years ago in reply to Dave Wilson

Guru 35595 points

Dave Wilson said:
I tend to think of NULL string and NULL pointer as the same thing

No, you shouldn't think that - it's wrong!

Dave Wilson said:
a NULL string could be a valid pointer to a buffer whose first byte is a 0.

Yes, that is correct.

0 Adam Brailove over 12 years ago in reply to Dave Wilson

Prodigy 135 points

Dave,

Can you please attach a link to where I might find your updated documentation.

thanks

0 Dave Wilson over 12 years ago in reply to Adam Brailove

TI__Mastermind 26900 points

Unfortunately, I can't - it resides only in source code in our SubVersion system right now! The documentation will appear in the next software release. Here's how I've worded things:

In the documentation for array g_sCmdTable, it now states:

"This is the command table that must be provided by the application. The last element of the array must be a structure whose pcCmd field contains a NULL pointer."

The documentation for CmdLineProcess() now contains the note:

"The command table is contained in an array named g_sCmdTable containing tCmdLineEntry structures which must be provided by the application. The array must be terminated with an entry whose pcCmd field contains a NULL pointer."

0 Dave Wilson over 12 years ago in reply to Andy Neil

TI__Mastermind 26900 points

I stand corrected!

0 Adam Brailove over 12 years ago in reply to Dave Wilson

Prodigy 135 points

OK, problem solved and tested- Thanks Dave.

For others, here is what I learned:

1) the command table needs a final terminating entry (this is currently not shown in the example code on p.24 of the "EK-LM4F120XL Firmware Development Package User's Guide", document# SW-EK-LM4F120XL-UG-9453

2) the final terminating element of the array should have as its first element a 0 (yielding a null pointer in memory), rather than a null string "" (which would yield a non-null pointer to a null string)

For example, use this:

tCmdLineEntry g_sCmdTable[] =

{

{ "foo", ProcessFoo, "The first command." },

{ "bar", ProcessBar, "The second command." },

{ "help", ProcessHelp, "Application help." },

{ 0, 0, "--terminating entry--" }

};

Do NOT use this:

{ "", 0, "--terminating entry--" }

Arm-based microcontrollers

Arm-based microcontrollers forum

Bug in Stellaris Firmware Development package - "utils/cmdline.c"