cam xpt_nounce_periph: Various fixes for periphs without a protocol
ClosedPublic
Actions

Authored by jhb on Jul 25 2023, 7:09 PM.

Details

Reviewers

imp
mav

Group Reviewers

cam

Commits

rG76b2e3907ced: cam xpt_*nounce_periph*: Various fixes for periphs without a protocol

Summary

If the periph doesn't have a valid protocol, these routines emit
fallback messages. However, the fallback messages duplicated the
periph name and unit number, and in the case of *denounce* included a
spurious newline.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 52911
Build 49802: arc lint + arc unit

Event Timeline

jhb created this revision.Jul 25 2023, 7:09 PM

Herald added a reviewer: cam. · View Herald TranscriptJul 25 2023, 7:09 PM

jhb requested review of this revision.Jul 25 2023, 7:09 PM

Harbormaster completed remote builds in B52825: Diff 125140.Jul 25 2023, 7:09 PM

Similar to D41176 and noticed while reading the code for that issue

Considering "%s%d: " is printed before, it should be removed here also. And the same should be done in xpt_denounce_periph_sbuf().

In xpt_announce_periph() and xpt_announce_periph_sbuf() it seems "%s%d: " is also excessive.

Agree with @mav as well: The '%s%d: ' and the args that feed them should be removed here and where he pointed out.
This is a rare corner case, but we should take care of it...
And the multiple printfs are somewhat racy.
I'm not sure why we have both routines still... oh, its for fallback sysctl... I wonder if we still need that given this has gone well as far as I know...

This revision is now accepted and ready to land.Jul 25 2023, 9:26 PM

jhb retitled this revision from cam xpt_denounce_periph: Trim spurious newline to cam xpt_*nounce_periph*: Various fixes for periphs without a protocol.Jul 31 2023, 8:40 PM

jhb edited the summary of this revision. (Show Details)

Fix additional issues noted by mav@

This revision now requires review to proceed.Jul 31 2023, 8:40 PM

Harbormaster completed remote builds in B52911: Diff 125392.Jul 31 2023, 8:40 PM

We should reimplement the non-sbuf versions by calling them with an sbuf that drains to printf.

In D41177#939499, @jhb wrote:

We should reimplement the non-sbuf versions by calling them with an sbuf that drains to printf.

Or just delete them... that was the next step...

In D41177#939547, @imp wrote:

In D41177#939499, @jhb wrote:

We should reimplement the non-sbuf versions by calling them with an sbuf that drains to printf.

Or just delete them... that was the next step...