Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert()

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jun 19, 2015 at 12:55:57PM +0200, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Fri, Jun 19, 2015 at 11:13:40AM +0100, Peter Maydell wrote:
> >> On 19 June 2015 at 11:07, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Fri, Jun 19, 2015 at 12:00:53PM +0200, Greg Kurz wrote:
> >> >> In case NDEBUG is defined, assert() expands to nothing and
> >> >> vhost_net_set_vnet_endian() doesn't get called...
> >> >>
> >> >> Suggested-by: Thomas Huth <thuth@redhat.com>
> >> >> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> >> >
> >> > Not sure what the point is.
> >> > We don't support building with NDEBUG.
> >> 
> >> Putting functional behaviour inside an assert() is still a really
> >> bad idea. If you're reading the code you probably skim over the
> >> assert() as not functionally relevant...
> >> 
> >> -- PMM
> >
> > I can apply this if commit log explains it's a readability
> > enhancement, not a bugfix.
> 
> Easy:
> 
> vhost_net: fix misuse of assert()
> 
> In case NDEBUG is defined, assert() expands to nothing and
> vhost_net_set_vnet_endian() doesn't get called...
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> 
> We don't support building with NDEBUG, but putting functional behaviour
> inside an assert() is still a really bad idea.  If you're reading the
> code you probably skim over the assert() as not functionally relevant...
> 
> Signed-off-by: Michael S. Tsirkin" <mst@redhat.com>

Pls submit v2 in the regular format - I can rewrite the commit log
but prefer not to, as it doesn't scale. There's no need to add my
signature though, git am -s does this automatically.

Thanks!
-- 
MST