Wireshark-dev: Re: [Wireshark-dev] Removing string constants from assertions
From: Evan Huus <eapache@xxxxxxxxx>
Date: Tue, 4 Sep 2012 17:33:30 -0400
On Tue, Sep 4, 2012 at 4:53 PM, Guy Harris <guy@xxxxxxxxxxxx> wrote:
>
> On Sep 4, 2012, at 1:39 PM, Evan Huus wrote:
>
>> I've noticed in several places the pattern of adding a message to
>> assertions in the form of a string constant:
>>
>> g_assert(condition && "explanation");
>>
>> This seems dangerous to me, primarily because if anyone ever mistypes
>> the && as a || then the assertion becomes dead code - it will always
>> pass.
>
> Yes, that's risky.
>
>> Also (though less important):
>> - it doesn't really add any benefit over simply putting the message in
>> a comment - a developer will have to open up the code anyways to
>> figure out what the problem is
>
> I'm not *entirely* sure that's the case; the message printed with the version of GLib on my machine (2.32.3) includes the entire assertion string:
>
>         ERROR:sourcefile.c:{lineno}:{func}: assertion failed: ({test condition} && "{string}")
>
> It Might Be Nice if there were a version of g_assert() that took two arguments - a condition and a description string - and printed
>
>         ERROR:sourcefile.c:{lineno}:{func}: assertion failed: {test condition} : {description})
>
> or something such as that, but there isn't.
>
> We might be able to hack up a macro of that sort (with a helper routine, just as the existing g_assert macros have g_assertion_message_ helper routines).

I mis-thought on that one. I meant to say that the developer will have
to open up the code anyways to *fix* the problem (and non-developers
probably don't care about the gory details behind an assertion
failure).

I agree that a macro would be nice, but I'm not sure how much of an
ugly hack it would end up being. I'm not too familiar with the bowels
of C macros though, so it could be easy.

I will convert the string constants to comments for now, and if
someone wants to write such a macro then they're free to grep through
for "g_assert" calls and fix them up.