freebsd-src icon indicating copy to clipboard operation
freebsd-src copied to clipboard

libsa: Add isprint()

Open kpowkitty opened this issue 6 months ago • 2 comments

Add isprint() alongside other isfoo() functions in libsa. Motivations for change:

  1. Future ACPICA work in loader needs it.
  2. libsa/hexdump.c already uses it.

Signed-off-by: Kayla Powell (AKA Kat) [email protected]

kpowkitty avatar Jun 27 '25 00:06 kpowkitty

Thank you for taking the time to contribute to FreeBSD! All issues resolved.

github-actions[bot] avatar Jun 27 '25 00:06 github-actions[bot]

Things look good.

I'm kinda on the fence about including the following: "libsa/hexdump.c uses it raw, will be replaced in a separate commit." in the first commit message. since you're going to immediately follow it up with a separate commit, I'm not sure that saying so and then having that commit be the next one adds value. If there were a rework that was coming some weeks or months in the future, it would make sense though. Can you remove it? Or do you have a reason I'm not seeing to include it?

So the rest looks great.

bsdimp avatar Jun 27 '25 05:06 bsdimp

Does this need an update to libsa.3 (the manual page?). If so, I can help!

concussious avatar Jul 07 '25 22:07 concussious

Does this need an update to libsa.3 (the manual page?). If so, I can help!

We do need it, but libsa.3 is also somewhat out of date and needs a lot of attention with someone that can read the code and see what's missing. It doesn't need to be with this commit, which I think is otherwise ready. Would you feel comfortable putting it into your list @concussious and looping me and @kpowkitty into the loop when you do? You can I can work on what's missing in libsa.4 as well, if you're wanting to really help a lot :).

bsdimp avatar Jul 09 '25 03:07 bsdimp

Automated message from ghpr: Thank you for your submission. This PR has been merged to FreeBSD's branch. These changes will appear shortly on our GitHub mirror.

bsdimp avatar Jul 23 '25 06:07 bsdimp