[PATCH v2 4/5] qmi-codegen: allow empty input fields

[Thomas Weißschuh]

On Sun, 2018-02-11T15:20+0100, Aleksander Morgado wrote:
> On Fri, Feb 9, 2018 at 10:07 PM, Thomas Weißschuh <thomas at t-8ch.de> wrote:
> > The LOC service allows empty messages to trigger the sending of certain
> > indicator messages.
> > ---
> >  build-aux/qmi-codegen/Message.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/build-aux/qmi-codegen/Message.py b/build-aux/qmi-codegen/Message.py
> > index 0bf62f1..57c0d5e 100644
> > --- a/build-aux/qmi-codegen/Message.py
> > +++ b/build-aux/qmi-codegen/Message.py
> > @@ -179,7 +179,7 @@ class Message:
> >          cfile.write(
> >              '\n'
> >              '    return self;\n')
> > -        if self.input.fields is not None:
> > +        if self.input.fields:
> 
> I'm confused here, why do we need the error_out code chunk if no TLVs
> may be given? The error_out case covers the logic when there is at
> least one mandatory TLV.

The patch tries removes the label.
The new code could have been written as:

	if self.input is not None and len(self.input):

The "if len(something):" is not really idiomatic and a plain "if something:"
does the same (in case of a list).
And "if self.input is not None and self.input" is equivalent to "if self.input".

Long story, short: We only need the label if we can jump to it.

> Also, wouldn't this be equivalent to just removing the if? I'm
> confused and not a python dev :) What I do see is that the error_out
> case is needed only if the logic previously added a goto error_out,
> which only happens a bit above in that code in another "if
> self.input.fields is not None:" check done. That means that removing
> the "is not None" from the if will actually not change the code at all
> (as there is no real goto error_out being added).

We need the if-block was the compiler will error out if we define a jump label
but not use it.

Actually I am now in favour of getting rid of "-Werror=unused-label".
What is your opinion?

Thomas

> 
> 
> >              cfile.write(
> >                  '\n'
> >                  'error_out:\n'
> > --
> > 2.16.1
> >
> 
> 
> 
> -- 
> Aleksander
> https://aleksander.es