why( warning: left-hand operand of comma expression has no effect)

Sam Bobroff sbobroff at shoretel.com
Mon Nov 26 01:17:02 CET 2012


On 25/11/12 13:50, Colin McCabe wrote:
> On Tue, Nov 20, 2012 at 2:17 PM, Sam Bobroff <sbobroff at shoretel.com> wrote:
>> On 20/11/12 21:34, Marc Lehmann wrote:
>>> On Tue, Nov 20, 2012 at 02:54:50AM +0000, Sam Bobroff <sbobroff at shoretel.com> wrote:
>>>> Sorry if you felt that I'd disrespected your code, that wasn't my
>>> A hack is something that happens to work, but is not well done.
>>>
>>>>> This use of assert is part of the C language that is well supported and in
>>>>> very common use for a long time now, and does what it is suppsoed to do
>>>>> very well and reliably. It's the only way to reliably achieve the effect
>>>>> libev needs.
>>>> Well... it's not an obvious, straight forward usage
>>> Very little in C is obvious and straight forward. It shouldn't be
>>> surprising that certain, even standard, idioms are not straightforward.
>>>
>>>> it seems to me that if the "normal usage" of assert was to print a
>>>> string message along with the test, assert() would take two arguments or
>>>> even a format string.
>>> Judging things by what they seem, but aren't, doesn't lead anywhere. By your
>>> logic, if "normal usage" of file systems would include directories, then C
>>> would surely have a mkdir function. But it doesn't have one.
>>>
>>> And btw., normal usage of assert is *always* to print a string message if
>>> the test fails. It's defined to do just that. If this isn't normal suage,
>>> what is?
>>>
>>>>> Putting error messages into a comment would completely defeat the purpose
>>>>> of having runtime error messages in the first place, namely telling whats
>>>>> wrong at *runtime*, when the comment is long gone.
>>>> I don't agree with you here: I think assert is for reporting "bugs in
>>>> the code"
>>> So you think assert is not for reporting runtime errors? Well, thats the only
>>> thing assert can do, because it evaluates the test at runtime only.
>>>
>>> It cannot report bugs in the code in any other way than by runtime error
>>> messages.
>>>
>>>> it always prints out the file and line number at which it occurred: this
>>>> information is mostly only useful to a programmer who has access to the
>>>> source code.
>>> A lot of asserts in libev, if they trigger, are mainly useful for me,
>>> and I don't generally have access to the sourcecode. It is much more
>>> informative to have human readable messages, and usually leads to much
>>> better feedback and understanding.
>>>
>>> Other asserts in libev indicate bugs in the caller code, and for these,
>>> human readable error messages are a must, at least by my standards.
>>>
>>> Your idea seems to be that all sourcecode is written by a single
>>> person. This is hardly the case in the real world. Human-readable error
>>> messages are a vast improvement over forcing people to read source code
>>> they might not even have access to (e.g. when libev wasn't compiled by
>>> them).
>>>
>>>> If they have access to the source and follow the assert message to where
>>>> it occurred, they would immediately see a code comment.  That was what I
>>>> was suggesting.
>>> And if not, the error message is useless. Great improvement indeed.
>>>
>>>> What I do disagree with, a little, is that assert() is the right way to
>>>> generate run-time error messages.
>>> Since that is the only thing it can do (creating run-time error messages),
>>> that means you think assert is not the right way to do anything, i.e. it
>>> is a useless function.
>>>
>>> This is clearly a minority opinion that needs strong evidence.
>>>
>>>> Assert is specific, in that it crashes your code with a core dump and
>>>> prints out the file and line number:
>>> C doesn't know anything about core dumps, and assert does not generally
>>> cause a core dump.
>>>
>>>> better, user friendly, message then I think you should do so via
>>>> fprintf() or similar, and follow up with an exit() or abort() only if
>>>> appropriate.
>>> fprintf has an obvious disadvantage: it cause quote noticable code bloat
>>> for a relatively rarely used feature and it pulls in the whole of stdio,
>>> which is an enourmous cost on amyn smaller systems.
>>>
>>> The only disadvantage of assert is that you don't like it without being able
>>> to explain why.
>>>
>>> assert wins trivially.
>>>
>> As you seem to already know everything, this conversation really can't
>> go anywhere.
>>
>> Goodbye, and again best of luck with libev.
>>
>> Sam.
> Hi Sam,
>
> You have to look at it from Marc's point of view.  From his point of
> view, having the assert compiled in to the program all of the time is
> a big advantage.  There are a lot of questions on this list that
> clearly relate to misuse of the libev APIs.  If the library fails
> silently with no error message, how can anyone start debugging those?
> Asking people to recompile the library to get debugging is not
> reasonable in my opinion.  Most people just take the version of libev
> that their Linux distribution (or ports program, or whatever)
> provides-- few of them know how to recompile or care to do so.
>
> I do think it would make sense to replace that:
> assert("foo was 0", foo) ;
> with:
> assert("foo was 0" && foo);
>
> The latter doesn't produce any gcc warnings for me, whereas the former
> does.  But at the end of the day, that's up to the maintainer... he's
> the one who has to sift through the compilation warnings.
>
> Colin
>
Hi Colin,

I did try to see things form Marc's point of view... but all I could see
was red ;-)

Please excuse me if I don't directly answer your points: I think pretty
much everyone will already agree on them and I do too: asserts are
important and good debugging is really important. I didn't intend to
imply otherwise.

Your suggestion of changing the comma operator to "&&" seems like a good
solution to me too but personally, as I sort of explained before, I
would use a more direct approach: assert() takes one argument and I want
it to take two, so I'd write my own function. Something like this:

#define ASSERT(SCALAR,MSG) assert2(__FILE__, __LINE__, #SCALAR,
(SCALAR), (MSG))

void assert2(const char *file, int line, const char *expr_string, int
expr_val, const char *msg) {
     if (expr_val)
         return;
     fprintf(stderr, "%s:%d Expression '%s' failed: %s\n", file, line,
expr_string, msg);
     abort();
}

Obviously we're just talking about opinion now, but I think this is more
straight-forward, and easier to understand, than either the comma
operator or double-ampersand operator solutions. Of course it's more
heavyweight too since you have to write a few lines of code.

(If you have problems with fprintf() or whatever then feel free to
implement it with direct calls to write() or whatever you like: that's
not important to the solution. I don't (at least off-hand) see any
particular disadvantages to this sort of solution and it has several
advantages: you could make the abort() call optional, format the message
however you like, allow printf formatting or not or whatever.)

By the way, did you notice that you made a mistake in your example?

> assert("foo was 0", foo) ;
This isn't going to compile: you need another set of parenthesis. I
wouldn't have pointed it out, because to do so is usually pointless
nit-picking, but in this case I think it's relevant because it adds
weight to my point that the original code is less than completely
obvious ;-)

Cheers,
Sam.

________________________________

This e-mail and any attachments are confidential. If it is not intended for you, please notify the sender, and please erase and ignore the contents.



More information about the libev mailing list