Few suggestions

Marc Lehmann schmorp at schmorp.de
Wed Jun 18 01:55:40 CEST 2008


On Tue, Jun 17, 2008 at 03:45:07PM -0700, nsf <no.smile.face at gmail.com> wrote:
> 1) Include pkg-config .pc file to source distribution.

patches welcome.

> 2) I wanted to compile my project (which is using libev) with -Wall and
> without any warnings, but there is few bad macros in libev.h. So, I was
> forced to remove all these macros and change them to ugly code.
> You can see in this commit what I mean:
> http://github.com/nsf/bmpanel/commit/60f74645be5e234f778d43621cbfc59d9f9d6cd8
> (just search with browser string "ev_io_init" and look around)

first, please read the section on warnings in the libev manual.

you didn't just change your code to just ugly code, you took (potentially)
*working* code and *broke* it, invoking undefined behaviour.

silencing warnings does not at all mean you fixed a problem - using the
macro creates correct code, your workaround silences the compiler and will
result in crashes (or worse) when you use a compiler intelligent enough to
optimize away your buggy code.

To fix bugs, one still has to use one's brain, which is why the compiler
issues warnings and not errors - because it cannot for sure know wether
this is an error or not.

As you can see from this example, it is easy to silence a compiler warning
by introducing bugs that the compiler cannot warn about anymore.

> I'm not a pro in C, but why are you using these type conversions in
> macros:

The ev_watcher cast is *required* for proper operation as the active and
pending members are not the same ones that libev uses internally.

The void * cast is used to silence an (in the case wrong) warning in gcc.
the latter is indeed not required by the C language rules (but if your
goal is to have less warnings, you should embrace them), but the former
certainly is.

> -----------------------------------------------
> #define ev_timer_set(ev,after_,repeat_)     do { ((ev_watcher_time *)(ev))->at = (after_); (ev)->repeat = (repeat_); } while (0)
> -----------------------------------------------
> 
> They will work without them.

That's a blatant lie :) They are _broken_ without them. Get a
(necessarilys, as average books don't cover this) good book about C and
learn about aliasing rules.

> And also there will be no warnings in -Wall mode.

Great. Compiler silenced, code broken.

Do yourself a favour and learn from the debian security disaster, where
exactly the same thing happened: warning/error message silenced, security
completely broken.

You have to *understand* error messages, not rearrange and break code so
it becomes too complex for the compiler to diagnose any problems.

> I'm using gcc 4.3.0.

gcc-4.3.0 is one of the compilers just intelligent enough in some cases to
actually break the intended meaning of your code (by optimising away your
= 0 assignments, as nothing accesses those fields).

If you want your code to work, you have to use -fno-strict-aliasing with
that version of gcc (because it isn't proper C anymore), or use the
provided macros, which are type-safe.

-- 
                The choice of a       Deliantra, the free code+content MORPG
      -----==-     _GNU_              http://www.deliantra.net
      ----==-- _       generation
      ---==---(_)__  __ ____  __      Marc Lehmann
      --==---/ / _ \/ // /\ \/ /      pcg at goof.com
      -=====/_/_//_/\_,_/ /_/\_\



More information about the libev mailing list