Its always important when starting a new project to ensure the foundations are solid. With PGPlus that invariably means tidying up the code and ensuring the layout is consistent.

Consider the following 2 functions:

void ADDSTACK(char *format,...)
{
va_list argum;
va_start(argum, format);
vsprintf(stack, format, argum);
va_end(argum);
stack = strchr(stack, 0);
}
void ENDSTACK(char *format,...)
{
va_list argum;
va_start(argum, format);
vsprintf(stack, format, argum);
va_end(argum);
stack = end_string(stack);
}

Both functions copy text at the position pointed to by stack, and then move that pointer forwards. However they both do so sub-optimally. We can  immediately improve this and remove the call to strchr by taking advantage of the fact that vsprintf returns the number of characters written.

void ADDSTACK(char *format,...)
{
va_list argum;va_start(argum, format);
stack += vsprintf(stack, format, argum);
va_end(argum);
}
void ENDSTACK(char *format,...)
{
va_list argum;va_start(argum, format);
stack += vsprintf(stack, format, argum);
va_end(argum);
stack = end_string(stack);
}

Looking at ENDSTACK we can see that it positions the stack pointer 1 place past the string terminator, so we can replace the call to end_string with a simple statement, like so:

void ENDSTACK(char *format,...)
{
va_list argum;
va_start(argum, format);
stack += vsprintf(stack, format, argum);
va_end(argum);
*stack++ = '';
}

And what we’ve ended up with is a variable argument call wrapping another variable argument call. Obviously thats a little silly, so instead of modifying the functions themselves, lets replace them with some preprocessor magic.

#define ADDSTACK(a, ...) stack += sprintf(stack, a, ## __VA_ARGS__)
#define ENDSTACK(a, ...) stack += sprintf(stack, a, ## __VA_ARGS__); *stack++ = ''

And now the compiler is doing a search and replace for us. However if we try to compile with that change, we’re going to immediately hit a compiler error:

channel.c: In function zc:
channel.c:637:3: error: else without a previous if
make: *** [../junk/channel.o] Error 1

Looking at the code in question reveals immediately what went wrong:

if (newchan)
ENDSTACK("<(%s)> ++ %s creates and joins this channel ++\n", p->zchannel, p->name);
else
ENDSTACK("<(%s)> ++ %s joins this channel ++\n", p->zchannel, p->name);

Our ENDSTACK macro creates 2 statements, and that breaks any blocks that arn’t wrapped in a pair of braces. Luckily theres an easy way to fix this which doesn’t involve editing lots of code. ENDSTACK moves the stack pointer 1 position past the end of the string its just written, so if we change the macro like so:

#define ENDSTACK(a, ...) stack += sprintf(stack, a, ## __VA_ARGS__) + 1

And with that our code compiles correctly without us needing to do any more work.

Advertisements