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.
Re: New santizer warning in KF 5.98 headers
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. On Tue, Jan 10, 2023 at 5:34 PM Albert Astals Cid wrote: > El dimarts, 10 de gener de 2023, a les 22:49:43 (CET), Michael Reeves va > escriure: > > /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. > > That code has been there since May last year, so not exactly "new". > > Given it doesn't seem to be crashing it would seem it's one of those > "undefined > but it works in all the compilers we care about". > > Of course patches to make the sanitizer are really welcome :) > > Cheers, > Albert > > > >
Re: New santizer warning in KF 5.98 headers
El dimarts, 10 de gener de 2023, a les 22:49:43 (CET), Michael Reeves va escriure: > /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. That code has been there since May last year, so not exactly "new". Given it doesn't seem to be crashing it would seem it's one of those "undefined but it works in all the compilers we care about". Of course patches to make the sanitizer are really welcome :) Cheers, Albert
New santizer warning in KF 5.98 headers
/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.
