Closed Bug 1095990 Opened 5 years ago Closed 5 years ago

Fix -Wnon-literal-null-conversion warnings in webrtc/signaling

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

clang is warning that a NUL character literal '\0' (FSMXFR_NULL_DIALSTRING) is passed to a function expecting a char pointer, so '\0' is treated as an integer zero which is treated as a NULL pointer. This char pointer parameter is copied using Cisco's "safe strncpy" function sstrncpy(), which ignores the NULL src parameter and initializes its dst out-parameter to a NUL-terminated zero-length string.

I believe the intention was for FSMXFR_NULL_DIALSTRING to NUL-terminated zero-length string itself.

media/webrtc/signaling/src/sipcc/core/gsm/fsmxfr.c:765:26 [-Wnon-literal-null-conversion] expression which evaluates to zero treated as a null pointer constant of type 'char *'
media/webrtc/signaling/src/sipcc/core/gsm/fsmxfr.c:1247:38 [-Wnon-literal-null-conversion] expression which evaluates to zero treated as a null pointer constant of type 'char *'
media/webrtc/signaling/src/sipcc/core/gsm/fsmxfr.c:1327:42 [-Wnon-literal-null-conversion] expression which evaluates to zero treated as a null pointer constant of type 'char *'
media/webrtc/signaling/src/sipcc/core/gsm/fsmxfr.c:1359:42 [-Wnon-literal-null-conversion] expression which evaluates to zero treated as a null pointer constant of type 'char *'
media/webrtc/signaling/src/sipcc/core/gsm/fsmxfr.c:2147:38 [-Wnon-literal-null-conversion] expression which evaluates to zero treated as a null pointer constant of type 'char *'
media/webrtc/signaling/src/sipcc/core/gsm/fsmxfr.c:2180:42 [-Wnon-literal-null-conversion] expression which evaluates to zero treated as a null pointer constant of type 'char *'
Attachment #8519513 - Flags: review?(rjesup)
Treat -Wnon-literal-null-conversion warnings as errors to match C++14's fix for C++11 core defect 903.

Mike, I know you're not a fan of the tree-wide -Werror=foo (bug 1090088 comment 12), but this clang warning will become a standard error in C++14. Patch #1 fixes the last of these warnings in mozilla-central, so we can treat this warning as an error now to prevent problems when C++14 comes along. (The webrtc/signaling code has had other -Wnon-literal-null-conversion warnings like bug 1001539, so regressions are not unlikely. :)

From the clang mailing list:

A goal of -Wnon-literal-null-conversion is to match rules proposed for C++14 (and possibly C++11 as a fix for core defect 903), and in those rules only integer literals (and nullptr) are valid null pointer constants.  The C++14 draft N3690 says "A null pointer constant is an integer literal (2.14.2) with value zero or a prvalue of type std::nullptr_t."

http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20140127/098329.html
Attachment #8519515 - Flags: review?(mh+mozilla)
Comment on attachment 8519513 [details] [diff] [review]
part-1-fix-Wnon-literal-null-conversion-warnings.patch

Review of attachment 8519513 [details] [diff] [review]:
-----------------------------------------------------------------

r+ because while the intent of the code was probably to pass NULL, not "" (whether it did so or not); there are many checks for NULL dialstrings in the code.  However, as pointed out this is equivalent to the current code, AND even more importantly we don't use the call transfer code at all, so this code will be eliminated anyways in the SDP rewrite currently under review.
Attachment #8519513 - Flags: review?(rjesup) → review+
Attachment #8519515 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/85c5327cc389
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.