On Tue, 5 Nov 2013, Marc Lehmann wrote:
On Mon, Nov 04, 2013 at 04:25:55PM -0500, Benjamin Kaduk <ka...@mit.edu> wrote:
I was doing some warning cleanup on a project that ships a bundled
version of libev (krb5), and figured that I should note the changes I
made in my working copy here, to see if they held interest for the
libev maintainers.
Please see the documentation section "COMPILER WARNINGS". What you are
trying to do is pretty pointless.
Thanks for the pointer, I am something of a libev newbie.
Though, what I did was clearly useful for my short-term goal: analyze my
own application's code. Eliminating warnings from imported code reduces
"noise" in the log so that I can get to the interesting parts which affect
my own code. There are lots of ways I could have done so, with patching
the code being just one of them. The whole point of my first mail was to
find out whether what I had done would additionally have long-term value.
That decision is yours, and I have no reason to try very much to affect
it. (I have no interest in maintaining a separate patchset atop libev just
to remove harmless warnings, as implied by the mindset implied by your
documentation.)
(3) The file-local function fd_rearm_all() is defined but never used.
It can safely be removed to eliminate this compiler warning.
It's used in most backends, actually, as a simple grep would have
shown. Eliminating it makes them not compile. Thats what you get for
blindly working around compiler warnings.
It was sufficient for my purpose at the time.
A simple grep would not be enough, one would need to find the part where
the different backend codes are included via the preprocessor instead of
being compiled as separate objects. The idiom of using the preprocessor to
include .c files is not one I encounter very often (the style guides in my
main projects frown on it); removing the 'static' keyword from the
function definition would probably help people like me who are unfamiliar
with the code gain a better/faster understanding of it (in addition to
eliminating the warning which drew my attention to it). Yes, it would
cause a symbol to escape the object's linkage, that's your tradeoff to
analyze. (It sounds like your decision is already made; please don't take
the above as an effort to sway your decision, it's just an explanation of
my mindset/thought process.)
I'll put links to the commits that I used to implement these fixes below.
At least your report indicates that clang can compile libev again - that's
progress.
Well, this is with the clang 3.2 rev 170710 included in the FreeBSD base
system from a svn snapshot from some time ago, compiling the embedded
libev 4.04 from the krb5 tree. Advances in either libev or clang could
have rendered this datapoint useless.
I will say once more that I do think that the parenthesis around the
bitwise operators (trimmed from the context) is very helpful for readers
of the code, and ask for its inclusion. The rest of these patches are not
really useful for anything other than removing warnings, and ignoring them
is fine by me.
Thanks for the input,
Ben
_______________________________________________
libev mailing list
libev@lists.schmorp.de
http://lists.schmorp.de/cgi-bin/mailman/listinfo/libev
