Aren't there are processor-specific assembler versions of memset for common chips? There are usually assembler versions (for example using REP MOVS on Intel which is light years faster) or even unrolled C versions (typically setting 8 or 16 bytes per iteration) that are used. I suspect this might be "fallback code" that doesn't actually get executed on any chips that anybody really uses.
Android Project Changeset 4f8b683: libc/memset.c
21–30 of 93 posts
Re: Android Project Changeset 4f8b683: libc/memset.c
#22The thing that shocks me about this bug is that it should have been caught when the compiler issued a warning about an unused function parameter. Either the warning was not enabled, or the programmer just ignored it; either of those options indicates a pretty severe lack of professionalism.
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.
Re: Android Project Changeset 4f8b683: libc/memset.c
#23Earlier quoted context omitted.
That sounds like a severe lack of perspective.
Huh? Any halfway decent software shop will have a policy of compiling with all warnings enabled, and fixing all sources of warnings (with only very rare exceptions). It is extremely unprofessional to disable or ignore warnings such as the "unused function parameter" warning. If you disagree, I respectfully submit that I would never let you near a C project that I was working on.
Re: Android Project Changeset 4f8b683: libc/memset.c
#24Earlier quoted context omitted.
That sounds like a severe lack of perspective.
Huh? Any halfway decent software shop will have a policy of compiling with all warnings enabled, and fixing all sources of warnings (with only very rare exceptions). It is extremely unprofessional to disable or ignore warnings such as the "unused function parameter" warning. If you disagree, I respectfully submit that I would never let you near a C project that I was working on.
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 agree that code should compile w/o warnings, but the benefits of tracking down and squelching unused function argument warnings are pretty marginal in my experience.
Re: Android Project Changeset 4f8b683: libc/memset.c
#25Can someone who lives in the world of C fill in the blanks? If this was a major typo, I have done things along those lines.
memset sets a block of memory to a given value. It is often used to initialize an uninitialized block of memory to 0. So often, in fact, that this bug where it always initialized the memory to 0 survived so long.
Re: Android Project Changeset 4f8b683: libc/memset.c
#26Forget 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…
Re: Android Project Changeset 4f8b683: libc/memset.c
#27The thing that shocks me about this bug is that it should have been caught when the compiler issued a warning about an unused function parameter. Either the warning was not enabled, or the programmer just ignored it; either of those options indicates a pretty severe lack of professionalism.
"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…
Re: Android Project Changeset 4f8b683: libc/memset.c
#28The thing that shocks me about this bug is that it should have been caught when the compiler issued a warning about an unused function parameter. Either the warning was not enabled, or the programmer just ignored it; either of those options indicates a pretty severe lack of professionalism.
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.
Re: Android Project Changeset 4f8b683: libc/memset.c
#29The thing that shocks me about this bug is that it should have been caught when the compiler issued a warning about an unused function parameter. Either the warning was not enabled, or the programmer just ignored it; either of those options indicates a pretty severe lack of professionalism.
"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…
Well, so then you're exposed to this error. Do:
void foo(int x)
{
(void)x;
}
to silence unused parameter warnings.Re: Android Project Changeset 4f8b683: libc/memset.c
#30Earlier quoted context omitted.
Huh? Any halfway decent software shop will have a policy of compiling with all warnings enabled, and fixing all sources of warnings (with only very rare exceptions). It is extremely unprofessional to disable or ignore warnings such as the "unused function parameter" warning. If you disagree, I respectfully submit that I would never let you near a C project that I was working on.
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…