[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream
From: |
Halil Pasic |
Subject: |
Re: [Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream |
Date: |
Wed, 13 Sep 2017 13:35:36 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 |
On 09/13/2017 11:53 AM, Cornelia Huck wrote:
> On Mon, 11 Sep 2017 18:36:00 +0200
> Halil Pasic <address@hidden> wrote:
>
>> On 09/06/2017 02:51 PM, Cornelia Huck wrote:
>>>>>> +void ccw_dstream_init(CcwDataStream *cds, CCW1 const *ccw, ORB const
>>>>>> *orb)
>>>>>> +{
>>>>>> + /*
>>>>>> + * We don't support MIDA (an optional facility) yet and we
>>>>>> + * catch this earlier. Just for expressing the precondition.
>>>>>> + */
>>>>>> + assert(!(orb->ctrl1 & ORB_CTRL1_MASK_MIDAW));
>>>>> I don't know, this is infrastructure, should it trust its callers? If
>>>>> you keep the assert, please make it g_assert().
>>>>
>>>> Why g_assert? I think g_assert comes form a test framework, this is not
>>>> test code.
>>> g_assert() is glib, no?
>>>
>>
>> It lives in GLib > GLib Utilities > Testing:
>> https://developer.gnome.org/glib/stable/glib-Testing.html
>>
>> The description of "Testing" starts like this: "GLib provides a framework
>> for writing and maintaining unit tests in parallel to the code they are
>> testing. The API is designed according to established concepts found in
>> the other test frameworks (JUnit, NUnit, RUnit), which in turn is based
>> on smalltalk unit testing concepts."
>>
>> So yes, it's both glib and testing framework. This is why I
>> ask why should one use g_assert in not-unit-test code.
>
> I have searched the archives, but unfortunately was not able to come to
> a conclusion. Checkpatch advises against using anything but g_assert or
> g_assert_not_reached in anything but test code, but that is because
> those other g_asserts can be made non-fatal. g_assert_not_reached does
> not seem to have a non-glib equivalent.
>
I think the standard library equivalent of g_assert_not_reached is
assert(false).
Yeah, checkpatch does not advise against using g_assert and
g_assert_not_reached in non-test code, but it does not advise against
using standard lib assert.
> I have it somewhere in the back of my mind that g_assert should be
> preferred...
>
OK, I will use g_assert.
- Re: [Qemu-devel] [PATCH 3/5] virtio-ccw: use ccw data stream, (continued)
[Qemu-devel] [PATCH 1/5] s390x/css: introduce css data stream, Halil Pasic, 2017/09/05
[Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Halil Pasic, 2017/09/05
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Cornelia Huck, 2017/09/06
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Halil Pasic, 2017/09/06
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Cornelia Huck, 2017/09/06
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Halil Pasic, 2017/09/06
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Cornelia Huck, 2017/09/07
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Janosch Frank, 2017/09/07
- Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Cornelia Huck, 2017/09/07
Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Dong Jia Shi, 2017/09/07
Re: [Qemu-devel] [PATCH 5/5] s390x/ccs: add ccw-tester emulated device, Cornelia Huck, 2017/09/07