GNOME Bugzilla – Bug 669479
various small fixes from static analysis
Last modified: 2012-02-13 13:44:49 UTC
My colleagues have fixed some problems in libsoup spotted by a static analysis tool (it was run on an older version, but I've verified that these patches are still applicable to master).
Created attachment 206890 [details] [review] [1/4] form: Check file pointer before dereferencing Author: Robert Swain <robert.swain@collabora.co.uk> [It's not documented as being allowed to be NULL, but later in the function there's a check, and GLib convention is generally that all 'out' parameters are allowed to be NULL whether it makes sense or not. -smcv] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Bug: https://bugzilla.gnome.org/show_bug.cgi?id=669479 Bug-NB: NB#297634 --- The actual patches are correctly-attributed git-format-patch output; I've mentioned the author here because it's not immediately visible otherwise. The long commit messages with [... -smcv] are from me, not the patch's author.
Created attachment 206891 [details] [review] [2/4] Safeguard against NULL in strcmp Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> [In both of these cases, the situation being guarded against is: check_password() is called, but soup_message_headers_get_one() does not find an "Authorization" header. -smcv] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Bug: https://bugzilla.gnome.org/show_bug.cgi?id=669479 Bug-NB: NB#297634
Created attachment 206892 [details] [review] [3/4] tests: simple-httpd: Don't leak index_path string From: Robert Swain <robert.swain@collabora.co.uk> Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Bug: https://bugzilla.gnome.org/show_bug.cgi?id=669479 Bug-NB: NB#297634
Created attachment 206893 [details] [review] [4/4] tests: sniffing: Avoid loop with uninitialised length From: Robert Swain <robert.swain@collabora.co.uk> [This would only happen if a request was made to an unhandled path, which this test doesn't do, but it seems reasonable to return an empty response rather than crashing. -smcv] Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Bug: https://bugzilla.gnome.org/show_bug.cgi?id=669479 Bug-NB: NB#297634
Comment on attachment 206890 [details] [review] [1/4] form: Check file pointer before dereferencing I'd say change the docs too; they already note that the function might be useful 'when you don't have any file upload controls, but are still using "multipart/form-data" anyway', and it's silly to require the caller to pass @file then, so we might as well make @file be officially optional.
(In reply to comment #5) > I'd say change the docs too and go ahead and commit it with that change
Created attachment 207074 [details] [review] soup_form_decode_multipart: document all (out) parameters as nullable Thanks, here's the additional patch I committed.
Created attachment 207075 [details] [review] soup_form_decode_multipart: allow file_control_name to be NULL as documented This was documented as allowed, but would have crashed. Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> https://bugzilla.gnome.org/show_bug.cgi?id=669479 --- I happened to spot this while fixing the documentation.
Created attachment 207076 [details] [review] soup_form_decode_multipart: check that msg really is a non-NULL message If it wasn't, the next line would segfault anyway. Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk> Bug: https://bugzilla.gnome.org/show_bug.cgi?id=669479
Thanks, fixed in git for 2.37.6