Re: [PATCH] gc: remove unused launchctl_get_uid() call

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 9, 2021, 11:53 UTC
Message-ID: <87bl52dkv1.fsf@evledraar.gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.2109091222260.59@tvgsbejvaqbjf.bet>

On Thu, Sep 09 2021, Johannes Schindelin wrote:

Show 21 quoted lines

> Hi Ævar,
>
> On Thu, 9 Sep 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> When the launchctl_boot_plist() function was added in
>> a16eb6b1ff3 (maintenance: skip bootout/bootstrap when plist is
>> registered, 2021-08-24), an unused call to launchctl_get_uid() was
>> added along with it. That call appears to have been copy/pasted from
>> launchctl_boot_plist().
>>
>> Since we can remove that, we can also get rid of the "result"
>> variable, whose only purpose was allow for the free() between its
>> assignment and the return. That pattern also appears to have been
>> copy/pasted from launchctl_boot_plist().
>
> I don't find the most crucial information in that commit message: what is
> the fall-out of the removal of this call?
>
> Such an analysis (_with_ a summary of it in the commit message) is
> definitely required. And it should not be left as an exercise for the
> reader.

Do you mean an assurance to the reader that the removed code doesn't have any side-effects? E.g. an addition of

    As the patch shows the returned value wasn't used at all in this
    function, the launchctl_get_uid() function itself just calls
    xstrfmt() and getuid(), neither of which have any subtle global
    side-effects, so this removal is safe.