1416286 - tls13con.c fix signed / unsigned compilation warnings

Status: NEW

(NSS :: Libraries, defect, P3)

Description

[jbonnafo]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:56.0) Gecko/20100101 Firefox/56.0
Build ID: 20171024165158

Steps to reproduce:

Using Visual studio 2017 compiler, compilation of tls3con.c generates warnings.




Actual results:

Generated warnings are:
security/nss/lib/ssl/tls13con.c:1669 [C4018] '<': signed/unsigned mismatch
security/nss/lib/ssl/tls13con.c:4135 [C4018] '>': signed/unsigned mismatch
security/nss/lib/ssl/tls13con.c:4140 [C4018] '<': signed/unsigned mismatch
security/nss/lib/ssl/tls13con.c:4401 [C4018] '>': signed/unsigned mismatch


Expected results:

Code should compile without any compiler warning.

Comment 1

[jbonnafo]

Another solution could be to change signature of tls13_Read0RttData function in order to have:

PRUint32
tls13_Read0RttData(sslSocket *ss, void *buf, PRUint32 len)

but this signature change would also impact some other functions.

Comment 2

[jbonnafo]

Hello,

Could you please assign this issue to me?

Thanks,

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

Hey Jean-Luc, this bug is already assigned to you :)

Comment 4

[jbonnafo]

Oh sorry, i should have noticed !

Comment 5

[jbonnafo]

On this one, i think the main item is about function:
PRInt32
tls13_LimitEarlyData(sslSocket *ss, SSL3ContentType type, PRInt32 toSend)

The signature of this function uses signed int whereas code of this function does comparisons with
PRUint32 earlyDataRemaining;

which causes compilation warnings.

tls13_LimitEarlyData is called once in sslsecur.c, with signed parameter.

I am not sure about the best route to follow to solve this warning.

Thanks,

Comment 6

[jbonnafo]

Hello,
i have posted a patch proposal changing also the signature of tls13_LimitEarlyData.
From what i have understood, the call tls13_LimitEarlyData is always called with >= 0 parameters.
Thanks,

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.