gcc -Werror=unused reports that update_desktop_file_entry::retval is never used.
This means that its callers always go into their error-handling code paths!
This may have consequences for correct service activation.
My fault, this regressed when I fixed Bug #33126 (consistently freeing things on both success and failure in that function).
However, neither of that function's callers treat errors differently, unless the error is OOM, so this has no practical effect.
I did spot a related memory leak while fixing it, though...
Created attachment 49378 [details] [review]
[PATCH 1/3] update_desktop_file_entry: don't leak file_path on one particular OOM
Revenge of #33126: most, but not all, temporary variables were freed on
this code path.
Created attachment 49379 [details] [review]
PATCH 2/3] update_desktop_file_entry: if the service name already existed, set error
If we're going to return FALSE for this (which has apparently always
been the case), then we should set an error properly.
Created attachment 49380 [details] [review]
[PATCH 3/3] update_desktop_file_entry: initialize return value properly, and actually return it
Since 1.4.4 (commit 75cfd97f) this function always returned FALSE. As far
as I can see this was actually harmless, because both of its callers
ignore any error that is not NoMemory (and treat it the same as success).
Review of attachment 49378 [details] [review]:
Review of attachment 49379 [details] [review]:
Review of attachment 49380 [details] [review]:
Ha. Looks good.
Fixed in git for 1.4.16 and 1.5.8, thanks