Closed Bug 1372065 Opened 3 years ago Closed 2 years ago

fix cppcheck linter error in netwerk/

Categories

(Core :: Networking, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: schien, Assigned: schien)

Details

Attachments

(1 file)

[netwerk/base/nsURLHelper.h:216]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.
[netwerk/base/nsURLHelper.h:220]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.
[netwerk/base/rust-url-capi/test/test.cpp:29]: (error) Common realloc mistake: 'mBuffer' nulled but not freed upon failure
[netwerk/cache2/CacheFileInputStream.cpp:148]: (error) syntax error
[netwerk/protocol/http/nsHttpHeaderArray.cpp:358]: (error) syntax error
[netwerk/streamconv/converters/nsHTTPCompressConv.cpp:297]: (error) Common realloc mistake: 'mInpBuffer' nulled but not freed upon failure
[netwerk/streamconv/converters/nsHTTPCompressConv.cpp:300]: (error) Common realloc mistake: 'mOutBuffer' nulled but not freed upon failure
[netwerk/test/gtest/TestStandardURL.cpp:205]: (error) syntax error
[netwerk/base/nsURLHelper.h:216]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.
[netwerk/base/nsURLHelper.h:220]: (error) Overflow in pointer arithmetic, NULL pointer is subtracted.

https://searchfox.org/mozilla-central/rev/d840ebd5858a61dbc1622487c1fab74ecf235e03/netwerk/base/nsURLHelper.h#216,220

NET_MAX_ADDRESS (-1) is used here to not specify string length.

The code here is not causing runtime issue because these two helper functions are only used by 
https://searchfox.org/mozilla-central/rev/d840ebd5858a61dbc1622487c1fab74ecf235e03/netwerk/protocol/http/nsHttpHandler.cpp#1905,1907, which guaranteed to only pass '\0' ended C-style string.

However |PrepareAcceptLanguages| should be rewrite using C++ style string operation to avoid such scary coding style.
[netwerk/base/rust-url-capi/test/test.cpp:29]: (error) Common realloc mistake: 'mBuffer' nulled but not freed upon failure
[netwerk/streamconv/converters/nsHTTPCompressConv.cpp:297]: (error) Common realloc mistake: 'mInpBuffer' nulled but not freed upon failure
[netwerk/streamconv/converters/nsHTTPCompressConv.cpp:300]: (error) Common realloc mistake: 'mOutBuffer' nulled but not freed upon failure

http://en.cppreference.com/w/c/memory/realloc

Null pointer will be returned but the original memory buffer will not be freed if |realloc| fails.
We should remember the original memory buffer and free it if error is detected.
[netwerk/test/gtest/TestStandardURL.cpp:205]: (error) syntax error

Seem a false-positive report.

https://searchfox.org/mozilla-central/rev/d840ebd5858a61dbc1622487c1fab74ecf235e03/netwerk/test/gtest/TestStandardURL.cpp#205
Assignee: nobody → schien
Whiteboard: [necko-active]
Bulk priority update: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258
Priority: -- → P1
Is the patch ready for review?
Status: NEW → ASSIGNED
Flags: needinfo?(schien)
Priority: P1 → P3
Whiteboard: [necko-active]
I'm evaluating whether we should spend more time on rewriting |PrepareAcceptLanguages| as mentioned in comment #1. Keep the ni? and will post my investigation later.
Rewriting |PrepareAcceptLanguages| can be a good first bug or a mentored bug, filed bug 1403802 for it.
Flags: needinfo?(schien)
Comment on attachment 8876559 [details]
Bug 1372065 - fix cppcheck linter error in Necko.

https://reviewboard.mozilla.org/r/147902/#review189570
Attachment #8876559 - Flags: review?(jduell.mcbugs) → review+
Pushed by schien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e0adce5b62a4
fix cppcheck linter error in Necko. r=jduell
https://hg.mozilla.org/mozilla-central/rev/e0adce5b62a4
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.