Re: [PATCH] Fix /proc pathname sizes on Solaris

[Rainer Orth <ro at CeBiTec dot Uni-Bielefeld dot DE>]

Hi Joel,

> On Mon, Sep 17, 2018 at 04:11:35PM +0200, Rainer Orth wrote:
>> I'm slowly working my way through the gdb patches from the
>> solaris-userland repo
>> 
>> 	https://github.com/oracle/solaris-userland/tree/master/components/gdb/patches
>> 
>> Some of them are pretty obvious and should be able to go in (such as
>> this one and the next), while others are either incomplete
>> (e.g. 008-syscalls.patch, which adds XML descriptions of the Solaris
>> syscalls, but lacks their registration) or inappropriate in their
>> current form (unnecessarily intrusive).
>> 
>> This one (001-fix-proc-name-size.patch) should be obvious given the
>> patches' comment:
>> 
>> # In Solaris, PID_MAX is 999999 (6 digit pid).
>> # In Solaris, lwpid_t is an unsigned int, so theoretically the lwp id
>> # could be 10 digits.
>> 
>> Two questions about procedure here:
>> 
>> * AFAIK Oracle has a corporate copyright assignment on file, so the
>>   patches should be covered.  Even if that were not the case, this one
>>   and the next are certainly below the 15-line limit for non-trivial
>>   patches.
>
> I checked, and indeed, Oracle has a copyright assignment.
>
>> * Given the code isn't mine, how should we handle attribution?  I
>>   suspect the engineer who committed the patch to github is the author,
>>   but don't know for certain.  Should I attribute it to her in the
>>   ChangeLog?
>
> Can you ask the user in question if they are the author? If not,
> can they help figuring out who it is? Ideally, we would want the
> name and email of the author of  the patch -- not sure what we should
> be doing if we don't have that info.

I've done some more digging myself: here's what I found:

* The MAX_PROC_NAME_SIZE part (done slightly differently) was originally
  done by Stefan Teleman when he imported gdb 7.6 into the userland repo
  (gdb.procfs.c.patch).

* The change to create_procinfo originated with April Chin when
  importing gdb 7.12.1 later (001-fix-proc-name-size.patch).

* I had to make minor adjustments for master to account for my removal
  of !NEW_PROC_API

So I'm going to attribute the patch to all three of us ;-)

>> @@ -483,7 +483,7 @@ create_procinfo (int pid, int tid)
>>      }
>>    else
>>      {
>> -      sprintf (pi->pathname, "/proc/%05d/lwp/%d", pid, tid);
>> +      sprintf (pi->pathname, "/proc/%d/lwp/%d", pid, tid);
>
> I am wondering how this ever worked for processes whose pid had
> fewer than 5 digits. I was initially concerned that this patch
> introduced a change of behavior that would create an incompatibility.
> But looking at Solaris 2.8 and 2.11 systems, I see processes with
> 3 or 4 digits PIDs, and the path in /proc doesn't have leading zeroes.

Indeed, and Solaris procfs doesn't care if the <pid> part contains
additional leading zeros or not.

> I also checked whether the file might be used on platforms other than
> Solaris (see configure.nat), and this does not appear to be the case.

True: I removed support for all other previous users when getting rid of
!NEW_PROC_API, IRIX and Tru64 UNIX support that had long been obsoleted.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University