[
https://issues.apache.org/jira/browse/HADOOP-10389?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14028136#comment-14028136
]
Haohui Mai commented on HADOOP-10389:
-------------------------------------
bq. printf and similar functions are not a problem because we have
_attribute_((format(printf))), which warns about cases where the format string
doesn't match the varargs. And we only ever use snprintf, vsnprintf, and the
other functions which print into a buffer of a known size.
Just a random grep, here is a call to {{vsprintf}} but not {{vsnprintf}}:
{code}
+ vsprintf(err->msg + class_len + SUFFIX_LEN, fmt, ap2);
+ va_end(ap2);
+ va_end(ap);
{code}
While trying to enforce good coding style is a good thing, it is a best-effort
claim but not a proof. I'm not trying to question your creditability on the
claim, as any human can make mistakes on such a large code base.
The point is the type checker in compiler gives you a proof automatically. With
proper uses, compilers do this job much better than humans, so please leave the
job to the compiler.
Additionally, the caller can easily trigger a buffer overflow by passing "%s"
as a format string. Alternatively, using {{stringstream}} will never have this
problem. Note that the code uses the c++ runtime anyway, why not just writing
the code in a type safe way and let the type system do the proof for you?
bq. I use valgrind to spot memory leaks.
valgrind is not a panacea. Valgrind is a dynamic tool where it spots the leak
only when the code runs into the buggy path. Many leaks happen in the
error-handling path, and I'm skeptical that (1) the unit tests are able to
cover all of them, (2) you'll ever be able to run valgrind in production
considering its overheads (10x~100x). Alternatively, the issue is largely
addressed in c++ compile time if the code passes smart pointers with discipline.
In short, they are best-effort approaches to address the common but critical
defects I've raised. With proper uses, a modern language like C++ is able to
provide much higher assurance (type checked proofs) in security and
reliability. Why not embracing these tools instead of putting heavy dependency
and responsibility on code review to catch these bugs?
> Native RPCv9 client
> -------------------
>
> Key: HADOOP-10389
> URL: https://issues.apache.org/jira/browse/HADOOP-10389
> Project: Hadoop Common
> Issue Type: Sub-task
> Affects Versions: HADOOP-10388
> Reporter: Binglin Chang
> Assignee: Colin Patrick McCabe
> Attachments: HADOOP-10388.001.patch, HADOOP-10389.002.patch,
> HADOOP-10389.004.patch, HADOOP-10389.005.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.2#6252)
