Bug#926896: sysvinit-utils: pidof is unreliable

[Jesse Smith]

On 2021-10-23 8:00 a.m.,
debian-init-diversity-request at chiark.greenend.org.uk wrote:
> Send Debian-init-diversity mailing list submissions to
> 	debian-init-diversity at chiark.greenend.org.uk
> 
> To subscribe or unsubscribe via the World Wide Web, visit
> 	http://www.chiark.greenend.org.uk/mailman/listinfo/debian-init-diversity
> 
> or, via email, send a message with subject or body 'help' to
> 	debian-init-diversity-request at chiark.greenend.org.uk
> 
> You can reach the person managing the list at
> 	debian-init-diversity-owner at chiark.greenend.org.uk
> 
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Debian-init-diversity digest..."
> 
> 
> Today's Topics:
> 
>    1. Bug#926896: sysvinit-utils: pidof is unreliable (Svante Signell)
>    2. Bug#926896: sysvinit-utils: pidof is unreliable (Jesse Smith)
>    3. Bug#926896: sysvinit-utils: pidof is unreliable (Thorsten Glaser)
> 
> 
> ----------------------------------------------------------------------
> 
> Message: 1
> Date: Fri, 22 Oct 2021 23:52:30 +0200
> From: Svante Signell <svante.signell at gmail.com>
> To: jsmith at resonatingmedia.com, 926896 at bugs.debian.org
> Subject: Bug#926896: sysvinit-utils: pidof is unreliable
> Message-ID: <fd8c0e688eb940851ce5bbb5a2e1481edeb2ffe9.camel at gmail.com>
> Content-Type: text/plain; charset="UTF-8"
> 
> Hi Jesse,
> 
> On Thu, 2021-10-21 at 14:51 -0300, Jesse Smith wrote:
>> Please give the attached patch a try and confirm it's working.  It's
>> working here for normal and zombie processes and it seems to be okay
>> for uninterruptable sleep processes too, but I'd like to have someone
>> else confirm everything looks right before I push this upstream.
> 
> Please don't use PATH_MAX in your code, that will eventually cause
> problems. Especially using PATH_MAX will make builds for GNU/Hurd to
> FTBFS.
> 
> Thanks!
> 
> 
> 
> ------------------------------
> 
> Message: 2
> Date: Fri, 22 Oct 2021 21:32:33 -0300
> From: Jesse Smith <jsmith at resonatingmedia.com>
> To: 926896 at bugs.debian.org
> Subject: Bug#926896: sysvinit-utils: pidof is unreliable
> Message-ID: <c56728c6-156e-da67-a813-8610632f7f85 at resonatingmedia.com>
> Content-Type: text/plain; charset=utf-8
> 
> On 2021-10-22 6:52 p.m., Svante Signell wrote:
>> Hi Jesse,
>>
>> On Thu, 2021-10-21 at 14:51 -0300, Jesse Smith wrote:
>>> Please give the attached patch a try and confirm it's working.  It's
>>> working here for normal and zombie processes and it seems to be okay
>>> for uninterruptable sleep processes too, but I'd like to have someone
>>> else confirm everything looks right before I push this upstream.
>> Please don't use PATH_MAX in your code, that will eventually cause
>> problems. Especially using PATH_MAX will make builds for GNU/Hurd to
>> FTBFS.
> 
> I inherited it that way. Though in this case it doesn't break for GNU
> Hurd systems because there is explicitly a check for that and, if it's
> not defined, PATH_MAX is declared in the code. So this code is GNU Hurd
> safe.
> 
> Jesse
> 
> 
> 
> ------------------------------
> 
> Message: 3
> Date: Sat, 23 Oct 2021 02:43:46 +0200 (CEST)
> From: Thorsten Glaser <t.glaser at tarent.de>
> To: jsmith at resonatingmedia.com, 926896 at bugs.debian.org
> Subject: Bug#926896: sysvinit-utils: pidof is unreliable
> Message-ID: <78682afd-cbad-84b2-978a-35318b68fdb1 at tarent.de>
> Content-Type: text/plain; charset=UTF-8
> 
> On Fri, 22 Oct 2021, Jesse Smith wrote:
> 
>> Hurd systems because there is explicitly a check for that and, if it's
>> not defined, PATH_MAX is declared in the code. So this code is GNU Hurd
>> safe.
> 
> To what value? (Spoiler: 1024 is wrong. All other values are also wrong.)
> 
> PATH_MAX does not exist on the Hurd because it has no limits.
> 
> You should do things like check for get_current_dir_name(3) in libc
> and use it if it is present, and dynamically allocate all pathname
> storage instead of hardcoding the maximum length.
> 

This is getting bogged down in the weeds and not related to whether the
readlink() patch works or not. The PATH_MAX usage is unrelated to the fix.

However, since you asked, PATH_MAX is set to 2048 in pidof.

Using get_current_dir_name() is not a valid way to do it as it is not
portable across C libraries.

- Jesse