[Development] It could be a little bug

[Jiergir Ogoerg]

1) m_state is declared private, has no setter and one getter which returns
its value
(not a reference to it) - so apparently nobody outside the class can change
it.

2) Throughout the code the author before setting m_state to something
is checking if there are any errors in it (cleanState()), so it appears he
didn't
mean to use multiple/bitwise data in this var.

I think the author thought he'd need to store multiple values in it
but later changed his mind and just forgot to update the function to not
work with
bits any longer.

Anyway I don't care about this bug since it's pretty much never triggered,
let it
stay there, it makes the code look professorial and sophisticated.




On Fri, Nov 1, 2013 at 4:35 PM, Marc Mutz <marc.mutz at kdab.com> wrote:

> On Tuesday, October 29, 2013 01:21:08 Jiergir Ogoerg wrote:
> > Hi,
> > There's an enum:
> >
> > //==> code
> > enum TableState
> >     {
> >         UnsupportedLocale,
> >         EmptyTable,
> >         UnknownSystemComposeDir,
> >         MissingComposeFile,
> >         NoErrors
> >     };
> > //<== code
> >
> > and this:
> >
> > //==> code
> > bool cleanState() const { return ((m_state & NoErrors) == NoErrors); }
> > //<== code
> >
> > Shouldn't the latter be ?:
> > //==> code
> > bool cleanState() const { return (m_state == NoErrors); }
> > //<== code
> >
> > Found at
> >
> QTSRC/qtbase/src/plugins/platforminputcontexts/compose/generator/qtablegene
> > rator.h
>
> According to the standard, they are equivalent, since reading a value from
> an
> enum object that does have one of the declared enum values results in
> undefined
> behaviour. That said, we're violating that in many places in Qt, e.g. in
> QFlags, so it might be that the code in question also abuses m_state that
> way.
> in that case, the existing code would be more robust, but faces the
> compiler
> optimising away the & NoErrors due to the reason mentioned above.
>
> The correct fix is to check if m_state takes values not in TableState
> values
> and if so, change its type to int or uint, otherwise, apply your
> suggestion.
>
> HTH,
> Marc
>
>
> Thanks,
> Marc
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20131102/726c0056/attachment.html>