[vlc-devel] [PATCH] fix qt unsigned comparison warnings

Lyndon Brown jnqnfe at gmail.com
Fri Sep 18 23:07:19 CEST 2020


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