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]: Looks fine.
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
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.