Comment by klodolph | Hacker News Reader

Comment by klodolph | Hacker News Reader

Parent Full thread

"Pretty severe" lack of professionalism? It's not unprofessional not to turn the warnings all the way up and sift through each and every one. Some of us work code inherited from other people with different coding guidelines, and some of the warnings will generate loads of messages for code that is not only technically correct but stylistically correct as well. The unused parameter warning is one of the biggest offenders in this area. And if you have a hundred warnings for unused parameters, which ones do you pay attention to?

Another potential issue is that the code in the patch might almost never be called in the first place. Most libc implementations (three that I know of, where I've examined the "memset" source) have a "memset" like that as a fallback only for platforms without an assembly version.

I'd say the problem is that there are several different ways this error could have been detected sooner: warnings, test, static analysis tools, and review, but none of them worked. But again, it's also possible that the code is never called because assembly versions are always available.

| The unused parameter warning is one of the biggest offenders in this area. And if you have a hundred warnings for unused parameters, which ones do you pay attention to?

Well, so then you're exposed to this error. Do:

    void foo(int x)
    {
       (void)x;
    }

to silence unused parameter warnings.

Go look at Google code search, which includes vast quantities of code written by professional teams and (often later) open sourced. Very little of it casts expressions to (void). When I started at Arbor, I got rid of all the (void) casts. What an eyesore.

Very little C code is const-correct either. Most of the C devs I know make fun of const-correctness.

| What an eyesore.

It's only an eyesore if you don't acknowledge the benefits that strict warnings can bring to the table.

| Most of the C devs I know make fun of const-correctness.

I for one don't mind putting in a tiny bit of upfront effort so that my compiler can double-check my work. Do the C devs you know also make fun of assert?

I don't acknowledge the benefits of casting things to (void). Therefore: definitely an eyesore.

And, no. Nobody makes fun of assert. Which rather makes my point for me.

If assertions disappear in certain optimized builds then what do you do, or recommend, to deal with warnings related to unused parameters or values if you don't believe that casting to void has value?

I don't understand how the question you ask even follows from the premise you forwarded. The fact that memset is probably always dead code here is why it's not really a bug. The fact that warnings about unused return values are useless 100-1000 times for every time they're potentially useful is why I don't litter my code with pointless casts.

As I pointed out above, in gcc you can explicitly mark variables as potentially unused.

And what do you suggest for MSVC, which doesn't support attribute unused or have any comparable declspecs?

If what you say is true, then you have to either put up with the warnings, compile at a level that doesn't have the warnings, or figure out a hack around them - I don't like any of those options.

I'm not as dogmatic as some others are in the thread. I just think it's a good idea to be warned when a variable is unused, enough that I'll mark my very few that might be #ifdef'ed out. If you're using a compiler that doesn't let you mark variables as such... well that sucks. You do what you can.

Your compiler may offer a more formal way to indicate that it's acceptable for a parameter to be unused. In gcc it's an attribute:

int foobar(int foo, int bar __attribute__((unused))) { /* Code that does not use bar */}

If you're using gcc, you can say __attribute__((unused)): http://gcc.gnu.org/onlinedocs/gcc-3.3.1/gcc/Variable-Attribu...

(void)x does not work on all compilers to silence the warning. gcc + cl != C.

So what if the code doesn't get called? That doesn't prevent it from being built. Considering that memset() is part of libc, which is a shared library, presumably it's going to get compiled regardless of whether it's called.

Current codebase: 9734 warnings.

As a point of pride, none of them were created by me, but it is what it is. Some of them come from since deprecated functions and nobody wants to go back and mess with old code; some of them come from sloppiness; some of them come from java's collection handling.

And yet this codebase gets a lot of work accomplished anyway.

Reply on news.ycombinator.com