[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert()
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert() |
Date: |
Fri, 19 Jun 2015 16:00:05 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
"Michael S. Tsirkin" <address@hidden> writes:
> On Fri, Jun 19, 2015 at 12:55:57PM +0200, Markus Armbruster wrote:
>> "Michael S. Tsirkin" <address@hidden> writes:
>>
>> > On Fri, Jun 19, 2015 at 11:13:40AM +0100, Peter Maydell wrote:
>> >> On 19 June 2015 at 11:07, Michael S. Tsirkin <address@hidden> 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 <address@hidden>
>> >> >> Signed-off-by: Greg Kurz <address@hidden>
>> >> >
>> >> > 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 <address@hidden>
>> Signed-off-by: Greg Kurz <address@hidden>
>>
>> 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" <address@hidden>
>
> 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.
I *hate* it when maintainers rewrite my commit message, then claim it's
mine :)
But that's not what I suggested! I suggested to append the additional
explanation you want after Greg's S-o-B. Makes it obvious that it's
yours.
I think that scales just fine in a simple case like this where you
already know the explanation.
- [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Greg Kurz, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Michael S. Tsirkin, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Peter Maydell, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Michael S. Tsirkin, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Markus Armbruster, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Michael S. Tsirkin, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(),
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Michael S. Tsirkin, 2015/06/19
- Re: [Qemu-devel] [PATCH] vhost_net: fix misuse of assert(), Greg Kurz, 2015/06/19