<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Feb 11, 2018 at 9:54 PM, Thomas Weißschuh <span dir="ltr"><<a href="mailto:thomas@t-8ch.de" target="_blank">thomas@t-8ch.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Sun, 2018-02-11T21:10+0100, Aleksander Morgado wrote:<br>
> On Sun, Feb 11, 2018 at 7:14 PM, Thomas Weißschuh <<a href="mailto:thomas@t-8ch.de">thomas@t-8ch.de</a>> wrote:<br>
>> On Sun, 2018-02-11T15:20+0100, Aleksander Morgado wrote:<br>
>>> On Fri, Feb 9, 2018 at 10:07 PM, Thomas Weißschuh <<a href="mailto:thomas@t-8ch.de">thomas@t-8ch.de</a>> wrote:<br>
>>>> The LOC service allows empty messages to trigger the sending of certain<br>
>>>> indicator messages.<br>
>>>> ---<br>
>>>> build-aux/qmi-codegen/Message.<wbr>py | 2 +-<br>
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)<br>
>>>><br>
>>>> diff --git a/build-aux/qmi-codegen/<wbr>Message.py b/build-aux/qmi-codegen/<wbr>Message.py<br>
>>>> index 0bf62f1..57c0d5e 100644<br>
>>>> --- a/build-aux/qmi-codegen/<wbr>Message.py<br>
>>>> +++ b/build-aux/qmi-codegen/<wbr>Message.py<br>
>>>> @@ -179,7 +179,7 @@ class Message:<br>
>>>> cfile.write(<br>
>>>> '\n'<br>
>>>> ' return self;\n')<br>
>>>> - if self.input.fields is not None:<br>
>>>> + if self.input.fields:<br>
>>><br>
>>> I'm confused here, why do we need the error_out code chunk if no TLVs<br>
>>> may be given? The error_out case covers the logic when there is at<br>
>>> least one mandatory TLV.<br>
>><br>
>> The patch tries removes the label.<br>
>> The new code could have been written as:<br>
>><br>
>> if self.input is not None and len(self.input):<br>
>><br>
>> The "if len(something):" is not really idiomatic and a plain "if something:"<br>
>> does the same (in case of a list).<br>
>> And "if self.input is not None and self.input" is equivalent to "if self.input".<br>
>><br>
><br>
> Aha, understood<br>
><br>
>> Long story, short: We only need the label if we can jump to it.<br>
>><br>
><br>
> Yes we agree on that<br>
><br>
>>> Also, wouldn't this be equivalent to just removing the if? I'm<br>
>>> confused and not a python dev :) What I do see is that the error_out<br>
>>> case is needed only if the logic previously added a goto error_out,<br>
>>> which only happens a bit above in that code in another "if<br>
>>> self.input.fields is not None:" check done. That means that removing<br>
>>> the "is not None" from the if will actually not change the code at all<br>
>>> (as there is no real goto error_out being added).<br>
>><br>
>> We need the if-block was the compiler will error out if we define a jump label<br>
>> but not use it.<br>
>><br>
>> Actually I am now in favour of getting rid of "-Werror=unused-label".<br>
>> What is your opinion?<br>
>><br>
><br>
> I believe we agree on how it should look like: no label defined and no<br>
> goto label OR label defined and goto label. If I'm not reading it<br>
> wrong, that was what the code was doing before this change, as both<br>
> the label and goto label were being defined over the same if condition<br>
> ( if self.input.fields is not None). But after the change one if()<br>
> condition and the other are different.<br>
><br>
> How about changing both to "if self.input.fields"? (i.e. instead of<br>
> one still saying if self.input.fields is not None)<br>
<br>
</div></div>My current version now just always adds the label but marks it with<br>
"G_GNUC_UNUSED". This way we can keep all the logic in one place.<br>
<br>
This is actually the exact usecase for the "unused" attribute on labels[0]:<br>
<br>
unused<br>
This feature is intended for program-generated code that may contain<br>
unused labels, but which is compiled with -Wall. It is not normally<br>
appropriate to use in it human-written code, though it could be useful<br>
in cases where the code that jumps to the label is contained within an<br>
#ifdef conditional.<br>
<br>
What do you think?<br>
<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace">Don't know, I think I would personally still prefer to not issue the label if it's not going to be needed. What is the problem with doing that?</div></div></div><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Aleksander<br><a href="https://aleksander.es" target="_blank">https://aleksander.es</a></div>
</div></div>