<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
On 04/26/2012 04:46 AM, Eric Blake wrote:
<blockquote cite="mid:4F9862A1.5020507@redhat.com" type="cite">
<pre wrap="">On 04/25/2012 02:01 PM, Laine Stump wrote:
</pre>
<blockquote type="cite">
<pre wrap="">This patch resolves <a class="moz-txt-link-freetext" href="https://bugzilla.redhat.com/show_bug.cgi?id=815270">https://bugzilla.redhat.com/show_bug.cgi?id=815270</a>
The function virNetDevMacVLanVPortProfileRegisterCallback() takes an
arg "virtPortProfile", and was checking it for non-NULL before using
it. However, the prototype for
virNetDevMacVLanPortProfileRegisterCallback had marked that arg with
ATTRIBUTE_NONNULL(). Contrary to what one may think,
ATTRIBUTE_NONNULL() does not provide any guarantee that an arg marked
as such really is always non-null; the only effect to the code
generated by gcc, is that gcc *assumes* it is non-NULL; this results
in, for example, the check for a non-NULL value being optimized out.
(Unfortunately, this code removal only occurs when optimization is
enabled, and I am in the habit of doing local builds with optimization
off to ease debugging, so the bug didn't show up in my earlier local
testing).
In general, virPortProfile might always be NULL, so it shouldn't be
marked as ATTRIBUTE_NONNULL. One other function prototype made this
same error, so this patch fixes it as well.
</pre>
</blockquote>
<pre wrap="">
Might be worth linking to
<a class="moz-txt-link-freetext" href="http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308">http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17308</a>
</pre>
<blockquote type="cite">
<pre wrap="">---
src/util/virnetdevmacvlan.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
</pre>
</blockquote>
<pre wrap="">
ACK. What an insidious bug.
As Laine and I discussed on IRC, I'm half wondering if we should just do:
#ifdef STATIC_ANALYSIS && /* attributes supported */
# define ATTRIBUTE_NONNULL(n) __attribute__((__nonnull__(n)))
#else
# define ATTRIBUTE_NONNULL(n) /* empty, due to gcc lameness */
#endif
so that our code will be pessimized under normal compiles, but _at
least_ places where we have bugs with improper use of the attribute
won't cause gcc to miscompile things; but still let us get NULL checking
when running clang or Coverity.
I also wonder if this has been detected by Coverity (checking a nonnull
parameter for NULL is dead code, which Coverity does tend to flag), and
we just haven't been following Coverity closely enough to notice.
</pre>
</blockquote>
Eric, I ran Coverity on current commit 'f78024b util: fix crash when
starting macvtap interfaces',<br>
Coverity hasn't complain this issue, although I also enabled
'--security' checkers in Coverity. <br>
<blockquote cite="mid:4F9862A1.5020507@redhat.com" type="cite">
<pre wrap="">
</pre>
<pre wrap="">
<fieldset class="mimeAttachmentHeader"></fieldset>
--
libvir-list mailing list
<a class="moz-txt-link-abbreviated" href="mailto:libvir-list@redhat.com">libvir-list@redhat.com</a>
<a class="moz-txt-link-freetext" href="https://www.redhat.com/mailman/listinfo/libvir-list">https://www.redhat.com/mailman/listinfo/libvir-list</a></pre>
</blockquote>
<br>
</body>
</html>