Re: New santizer warning in KF 5.98 headers
That looks a lot better. Weird magic numbers are what enums are ment to avoid. On Wed, Jan 11, 2023 at 7:26 AM Nicolas Fella wrote: > On 1/10/23 22:49, Michael Reeves wrote: > > /usr/include/KF5/KConfigWidgets/kstandardaction.h:261:64: runtime > > error: load of value 4294967295, which is not a valid value for type > > 'Qt::ConnectionType' > > > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior > > /usr/include/KF5/KConfigWidgets/kstandardaction.h:261:64 in > > > > The issue stems for assigning an int to a enum which is internally > > considered unsigned and possibly smaller than the four byte int. If > > this is doing what we expect than I need a way to shut off the warning. > Looks like > https://invent.kde.org/frameworks/kconfigwidgets/-/merge_requests/175 > aims to address this? >
Re: New santizer warning in KF 5.98 headers
On 1/10/23 22:49, Michael Reeves wrote: /usr/include/KF5/KConfigWidgets/kstandardaction.h:261:64: runtime error: load of value 4294967295, which is not a valid value for type 'Qt::ConnectionType' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/include/KF5/KConfigWidgets/kstandardaction.h:261:64 in The issue stems for assigning an int to a enum which is internally considered unsigned and possibly smaller than the four byte int. If this is doing what we expect than I need a way to shut off the warning. Looks like https://invent.kde.org/frameworks/kconfigwidgets/-/merge_requests/175 aims to address this?
Re: New santizer warning in KF 5.98 headers
On Mittwoch, 11. Januar 2023 11:26:42 CET Ingo Klöcker wrote: > On Mittwoch, 11. Januar 2023 11:02:04 CET Milian Wolff wrote: > > On Dienstag, 10. Januar 2023 23:45:26 CET Michael Reeves wrote: > > > Thanks. I would say your right there this would definitely have caught > > > someone's attention if didn't work in practice with what kde needs. > > > Santizers are by design quite pedantic as serves there purpose well. > > > > I agree, the code is clearly wrong and it's unclear what it's trying to > > achieve here. Does anyone know what this is trying to do? > > > > Qt::ConnectionType connectionType = static_cast(-1) > > > > Should this maybe just be changed to use Qt::AutoConnection? > > The code: > https://invent.kde.org/frameworks/kconfigwidgets/-/blob/master/src/ > kstandardaction.h#L253 > > What the code does: > The default value `static_cast(-1)` serves as hint that > the function should decide what kind of connection type to use and for some > reason the `ConfigureToolbars` action explicitly needs to use a > `Qt::QueuedConnection` instead of a `Qt::AutoConnection`. This could be > changed to a std::optional (for KF6) to make the intention clear. Indeed, I should have read the body of the function and not stop at the signature only :) Thanks for the explanation Ingo! And std::optional for KF6 would be great imo, if this is really needed. > Why the code does what it does: > One could question whether this special casing for `ConfigureToolbars` is > still necessary. The bug report about the crash that this seems to have > fixed is from 2009: https://bugs.kde.org/show_bug.cgi?id=200815 > > Regards, > Ingo -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.
Re: New santizer warning in KF 5.98 headers
On Mittwoch, 11. Januar 2023 11:02:04 CET Milian Wolff wrote: > On Dienstag, 10. Januar 2023 23:45:26 CET Michael Reeves wrote: > > Thanks. I would say your right there this would definitely have caught > > someone's attention if didn't work in practice with what kde needs. > > Santizers are by design quite pedantic as serves there purpose well. > > I agree, the code is clearly wrong and it's unclear what it's trying to > achieve here. Does anyone know what this is trying to do? > > Qt::ConnectionType connectionType = static_cast(-1) > > Should this maybe just be changed to use Qt::AutoConnection? The code: https://invent.kde.org/frameworks/kconfigwidgets/-/blob/master/src/ kstandardaction.h#L253 What the code does: The default value `static_cast(-1)` serves as hint that the function should decide what kind of connection type to use and for some reason the `ConfigureToolbars` action explicitly needs to use a `Qt::QueuedConnection` instead of a `Qt::AutoConnection`. This could be changed to a std::optional (for KF6) to make the intention clear. Why the code does what it does: One could question whether this special casing for `ConfigureToolbars` is still necessary. The bug report about the crash that this seems to have fixed is from 2009: https://bugs.kde.org/show_bug.cgi?id=200815 Regards, Ingo signature.asc Description: This is a digitally signed message part.
Re: New santizer warning in KF 5.98 headers
On Dienstag, 10. Januar 2023 23:45:26 CET Michael Reeves wrote: > Thanks. I would say your right there this would definitely have caught > someone's attention if didn't work in practice with what kde needs. > Santizers are by design quite pedantic as serves there purpose well. I agree, the code is clearly wrong and it's unclear what it's trying to achieve here. Does anyone know what this is trying to do? Qt::ConnectionType connectionType = static_cast(-1) Should this maybe just be changed to use Qt::AutoConnection? Thanks -- Milian Wolff m...@milianw.de http://milianw.de signature.asc Description: This is a digitally signed message part.
