[vlc-devel] [PATCH] fix qt unsigned comparison warnings
Pierre Lamot
pierre at videolabs.io
Mon Sep 21 09:45:37 CEST 2020
Hi,
this class extend QAbstractListModel, which uses uses signed indexes (in
QModelIndex).
negatives values represent invalid indexes.
regards,
On 2020-09-18 23:07, Lyndon Brown wrote:
> I have to disagree on this. The function parameter is an index, and it
> is good design to forbid signed values unless there's a good reason,
> such as -1 having a special meaning, (if such things cannot be easily
> communicated by other means). I see no legit reason for opening up
> passing in signed values here, which would then require implementing a
> check to reject negative values, and raise issues of UB wrt. signed
> value calculations and overflow.
>
> With respect, I don't think that the fact of callers using signed
> variables to handle the index passed to this function is a valid
> argument for changing the function to take a signed value.
>
> If indeed the calling code is handling signed indexes as you say (I
> haven't looked myself), then a question needs to be raised as to why;
> an assessment done as to whether that can be changed; and assessment
> done to ensure that a negative value is not incorrectly being passed to
> the function, which would cause a likely out of bounds access.
>
> Regards,
> Lyndon
>
> On Fri, 2020-09-18 at 09:37 +0200, Pierre Lamot wrote:
>> Hi, I think this should probably be the other way around
>>
>> The method is either called with as argument `int QModelIndex::row()`
>> or
>> an int index.
>> you should rather change the argument from unsigned to signed.
>>
>> On 2020-09-17 23:06, Lyndon Brown wrote:
>> > currently a ton of warnings are printed in the build output due to
>> > a
>> > couple of checks that treat an unsigned var as though signed. the
>> > attached patch fixes the problem.
>> >
>> > _______________________________________________
>> > vlc-devel mailing list
>> > To unsubscribe or modify your subscription options:
>> > https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list