Live data from Hacker News

Android Project Changeset 4f8b683: libc/memset.c

review.source.android.com

61–70 of 93 posts

Re: Android Project Changeset 4f8b683: libc/memset.c

#61
post #60
post #56

Earlier quoted context omitted.

If I understand you correctly, you think programmers should say: int i; for (i = 0; i Instead of: memset(buffer, 0, SIZE) Because it indicates better buffer management.

it should be memset(buffer,0,SIZE*sizeof(int))

Only if it's a buffer of ints. I had assumed it was buffer of unstructured memory, which would be chars and of 1 byte each.

Re: Android Project Changeset 4f8b683: libc/memset.c

#62
post #57
post #53

Earlier quoted context omitted.

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?

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?

Re: Android Project Changeset 4f8b683: libc/memset.c

#63
post #56
post #31

Earlier quoted context omitted.

I'll offer the tip that even if you have a fast memset, you can and should avoid using it when possible. Seeing memset is usually a sign that someone is being lazy about watching the length of their buffers and/or verifying their copies aren't off-by-one.

If I understand you correctly, you think programmers should say: int i; for (i = 0; i Instead of: memset(buffer, 0, SIZE) Because it indicates better buffer management.

I took it as saying you should remember how much of the buffer you have actually used. If you only read what you write, it doesn't matter whether the rest of the buffer is filled with \0 or not, so you can skip the memset.

Re: Android Project Changeset 4f8b683: libc/memset.c

#65

Earlier quoted context omitted.

"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 offend…

| 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.

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 */}

Re: Android Project Changeset 4f8b683: libc/memset.c

#66
post #30
post #24

Earlier quoted context omitted.

Any halfway decent software shop will have a policy of compiling with all warnings enabled FWIW, your definition of "halfway decent" would exclude most software shops in the world. It is extremely unprofessional to disable or ignore warnings such as the "unused function parameter" warning Unused function parameter warnings are often noise (e.g., due to #ifdefs, unimplemented APIs, backward compatible APIs, etc). I ag…

All I can say is than in 15 years (fuck.) I've never worked on a team that would have caught a bug like this in dead code. -Wall has been the gold standard on the teams I've worked on.

-Wall unfortunately leaves a lot of warnings disabled. We've used:

"-W -Wall -Wcast-align -Wstrict-prototypes -Wmissing -prototypes -Wpointer-arith -Wshadow -Wsign-compare -Wformat=2 -Wno-format-y2k -Wimplicit -Wmissing-braces -Wnested-externs -Wparentheses -Wtrigraphs"

Update: Forgot the most important one ... -Werror. Making warnings equivalent to errors is the only way to ensure that they're always taken care of.

Re: Android Project Changeset 4f8b683: libc/memset.c

#68
post #62
post #57

Earlier quoted context omitted.

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.

Re: Android Project Changeset 4f8b683: libc/memset.c

#69

Earlier quoted context omitted.

The "unused parameter" error is one of the most useless of C compiler errors. There are dozens of perfectly valid reasons why a parameter would go unused, so the occasional error just gets lost in the noise. It's more helpful in C++, which can omit parameter names to indicate that it's intentionally unused. But then you're using C++; not worth the tradeoff.

It's not hard to type a statement like "(void)v;" to explicitly say, "v is intentionally unused." I fail to see the drawback of this approach. Of course you should only do this when (1) you can't remove the parameter from the function's prototype and (2) there is a good reason for it to be unused. But once you develop that habit, the compiler can help you catch bugs that are otherwise easy to miss. Sounds like a wort…

What you want is __attribute__((unused)), assuming you're compiling with gcc.

Re: Android Project Changeset 4f8b683: libc/memset.c

#70
post #17

Forget the typo. This is the slowest, most naive implementation of memset possible. Is this actual shipping code? I've followed the changeset up to the top and it looks like it's in platform/bootable/bootloader/legacy. Hopefully this isn't actually called anywhere, but the review date says May 13th. What's the point of optimizing the Java VM when functions like memset, malloc, memcmp, memcpy, memchr, etc, etc haven't…

Because in boot code you generally don't care about speed, but size is a very important factor. I've personally had to rewrite standard library functions to be intentionally small and stupid just so a piece of boot code could fit into ROM. It's not fun, but there is a certain satisfaction when you rewrite putchar and think, I just saved 10k of code!

> Because in boot code you generally don't care about speed, but size is a very important factor.

why can't you have a '#ifdef PREFER_SIZE_OVER_SPEED' or something similar ?

Post reply on HN