1446100 - build warning: "shlibsign.c:151:22: warning: comparison of integers of different signs: 'int' and 'CK_ULONG' (aka 'unsigned long') [-Wsign-compare]"

Status: RESOLVED FIXED

(NSS :: Tools, enhancement)

Description

[Daniel Holbert [:dholbert]]

Build warning (which I encountered while compiling mozilla-central, but it affects standalone NSS, too):
=====
security/nss/cmd/shlibsign/shlibsign.c:151:22: warning: comparison of integers of different signs: 'int' and 'CK_ULONG' (aka 'unsigned long') [-Wsign-compare]
     if (bytesWritten != ulValueLen) {
         ~~~~~~~~~~~~ ^  ~~~~~~~~~~
=====

This is checking the return value from PR_Write. Here's a bit more context:
static PRStatus
writeItem(PRFileDesc *fd, CK_VOID_PTR pValue,
          CK_ULONG ulValueLen, char *file)
{
    int bytesWritten;
[...]
    bytesWritten = PR_Write(fd, pValue, ulValueLen);
    if (bytesWritten != ulValueLen) {
        lperror(file);
        return PR_FAILURE;
    }
    return PR_SUCCESS;
}
https://hg.mozilla.org/projects/nss/file/tip/cmd/shlibsign/shlibsign.c#l151


Technically we should be testing whether bytesWritten is negative (to see if it failed), and then casting it to be unsigned & and comparing it to see if it's as much as the unsigned-int that we expected to write. See documentation here:
https://dxr.mozilla.org/mozilla-central/source/nsprpub/pr/include/prio.h#724-727
...and this reference callsite in mozilla-central that seems to be doing the right thing:
https://dxr.mozilla.org/mozilla-central/rev/6ff60a083701d08c52702daf50f28e8f46ae3a1c/dom/media/gmp/GMPServiceParent.cpp#1049-1052

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: fix v1 (now with more lines of context)

Comment 3

[Kai Engert (:KaiE:)]

Comment on attachment 8959278 [details] [diff] [review]
fix v1 (now with more lines of context)

makes sense, thanks for identifying the fix!

Comment 4

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/projects/nss/rev/35f951089cbc

Comment 5

[Daniel Holbert [:dholbert]]

Thanks!