Bug 273966 - libprocstat: use elf_getphdrnum() rather than deprecated elf_getphnum()
Summary: libprocstat: use elf_getphdrnum() rather than deprecated elf_getphnum()
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Ed Maste
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-09-20 13:18 UTC by John Hein
Modified: 2023-09-26 19:06 UTC (History)
3 users (show)

See Also:
jcfyecrayz: mfc-stable14?
jcfyecrayz: mfc-stable13?
jcfyecrayz: mfc-stable12?


Attachments
[patch] use non-deprecated elf_getphdrnum() rather than elf_getphnum() (472 bytes, patch)
2023-09-20 13:26 UTC, John Hein
no flags Details | Diff
[patch] v2: use non-deprecated elf_getphdrnum() rather than elf_getphnum() (473 bytes, patch)
2023-09-20 13:50 UTC, John Hein
no flags Details | Diff
[patch] v3: use non-deprecated elf_getphdrnum() rather than elf_getphnum() (764 bytes, application/mbox)
2023-09-22 01:41 UTC, John Hein
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Hein 2023-09-20 13:18:17 UTC
libprocstat should probably be updated to use the non-deprecated elf_getphdrnum() rather than elf_getphnum() (marked as deprecated).

The implementations are identical in elftoolchain.

elf_getphnum does not exist in elfutils - and has never existed.  The similar function elf_getshnum() was deprecated in elfutils in favor of elf_getshdrnum() in 2009.  This deprecation occurred just before elf_getphdrnum() was added - also in 2009.

This could be merged to all supported FreeBSD versions (i.e., back to 12.x - and even beyond that).
Comment 1 John Hein 2023-09-20 13:26:35 UTC
Created attachment 245052 [details]
[patch] use non-deprecated elf_getphdrnum() rather than elf_getphnum()
Comment 2 John Hein 2023-09-20 13:50:58 UTC
Created attachment 245056 [details]
[patch] v2: use non-deprecated elf_getphdrnum() rather than elf_getphnum()

The implementation of elf_getphnum() and elf_getphdrnum() are not quite identical.  The return value for errors is 0 for the former and -1 for the latter.

Patch adjusted accordingly.
Comment 3 Ed Maste freebsd_committer freebsd_triage 2023-09-20 14:35:06 UTC
LGTM

can you either attach a formatted git patch to preserve author metadata, or just let me know what I should use for `git commit --author=...`
Comment 4 John Hein 2023-09-22 01:41:31 UTC
Created attachment 245102 [details]
[patch] v3: use non-deprecated elf_getphdrnum() rather than elf_getphnum()

(In reply to Ed Maste from comment #3)
'git format-patch' flavor attached.
Comment 5 commit-hook freebsd_committer freebsd_triage 2023-09-22 01:48:45 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=633094c27f0ac1b1001d5bd24a883240b4bce1dc

commit 633094c27f0ac1b1001d5bd24a883240b4bce1dc
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2023-09-21 23:43:05 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-22 01:47:26 +0000

    libprocstat: use elf_getphdrnum rather than deprecated elf_getphnum

    PR:             273966
    Reviewed by:    emaste

 lib/libprocstat/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 6 Oleg Sidorkin 2023-09-23 15:20:23 UTC
(In reply to commit-hook from comment #5)

stabls/14 seems to have the same problem. Are there any plans to MFC?
Comment 7 John Hein 2023-09-23 15:57:18 UTC
(In reply to Oleg Sidorkin from comment #6)
Yeah, this was mentioned in comment 0, but now I officially set the mfc flags here to request the MFC.

This can be merged back to 9, actually (8 had procstat, but no libprocstat).  elf_getphnum has been deprecate for a long time.
Comment 8 commit-hook freebsd_committer freebsd_triage 2023-09-25 13:46:41 UTC
A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fd8bf2ecc05af841aa7e8369a43861cdc122d404

commit fd8bf2ecc05af841aa7e8369a43861cdc122d404
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2023-09-21 23:43:05 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-25 13:45:06 +0000

    libprocstat: use elf_getphdrnum rather than deprecated elf_getphnum

    PR:             273966
    Reviewed by:    emaste

    (cherry picked from commit 633094c27f0ac1b1001d5bd24a883240b4bce1dc)

 lib/libprocstat/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 9 Ed Maste freebsd_committer freebsd_triage 2023-09-25 13:59:33 UTC
(In reply to Oleg Sidorkin from comment #6)
There's no real need to merge this further back; even though elf_getphnum is deprecated it exists in those older branches and will not be removed.


elf_getphnum is:

/* Deprecated API */
int
elf_getphnum(Elf *e, size_t *phnum)
{
        return (_libelf_getphdrnum(e, phnum) >= 0);
}
Comment 10 John Hein 2023-09-26 11:25:03 UTC
(In reply to Ed Maste from comment #9)
"There's no real need to merge this further back; even though elf_getphnum is deprecated it exists in those older branches and will not be removed."

At least one reason exists.  libelf from devel/elfutils provides elf_getphdrnum, but not elf_getphnum().  So linking that libelf with libprocstat triggers link errors (undefined symbol).  See bug 273845.

So MFC to 12 & 13 can be helpful.  A new 12 release may never happen, but 12-stable users will appreciate it.
Comment 11 commit-hook freebsd_committer freebsd_triage 2023-09-26 13:05:50 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=09cd74a0b8d5a0b625ac7706ddc4655664fb05f9

commit 09cd74a0b8d5a0b625ac7706ddc4655664fb05f9
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2023-09-21 23:43:05 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-26 13:04:40 +0000

    libprocstat: use elf_getphdrnum rather than deprecated elf_getphnum

    PR:             273966
    Reviewed by:    emaste

    (cherry picked from commit 633094c27f0ac1b1001d5bd24a883240b4bce1dc)
    (cherry picked from commit fd8bf2ecc05af841aa7e8369a43861cdc122d404)

 lib/libprocstat/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 12 commit-hook freebsd_committer freebsd_triage 2023-09-26 13:05:51 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=205f9a4fbc82bdf8c8a7e5de4ef053c238d4769e

commit 205f9a4fbc82bdf8c8a7e5de4ef053c238d4769e
Author:     John Hein <jcfyecrayz@liamekaens.com>
AuthorDate: 2023-09-21 23:43:05 +0000
Commit:     Ed Maste <emaste@FreeBSD.org>
CommitDate: 2023-09-26 13:05:16 +0000

    libprocstat: use elf_getphdrnum rather than deprecated elf_getphnum

    PR:             273966
    Reviewed by:    emaste

    (cherry picked from commit 633094c27f0ac1b1001d5bd24a883240b4bce1dc)
    (cherry picked from commit fd8bf2ecc05af841aa7e8369a43861cdc122d404)

 lib/libprocstat/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 13 Ed Maste freebsd_committer freebsd_triage 2023-09-26 13:06:42 UTC
(In reply to John Hein from comment #10)
Ah, I wasn't aware that base system libprocstat was being used w/o base libelf. I've merged to stable/13 and stable/12 now.
Comment 14 John Hein 2023-09-26 19:06:37 UTC
(In reply to Ed Maste from comment #13)
Thanks, Ed.  Linking a non-base libelf is probably fairly rare, but it can be valuable to be able to do so.