kldxref: skip .pkgsave files

[freebsd_igalic.co]

Summary

add .pkgsave to list of skipped files, by skipping all files that
contain two '.'

Motivation: This should help people transitioning from
traditional setups to pkgbase experience a lot less friction.

Test Plan

update your system to pkgbase (using, for instance https://alpha.pkgbase.live/ ;)
or, copy .ko files in /boot/kernel to .ko.pkgsave

service kldxref onerestart
reboot, see unhappy boot messages

apply this patch
rebuild kldxref
reboot and see *no* unhappy boot messages!

[emaste]

LGTM even though we're probably close to the point where it makes sense to have a loop with a list of extensions to ignore.

[freebsd_igalic.co]

In D27959#624564, @emaste wrote:

LGTM even though we're probably close to the point where it makes sense to have a loop with a list of extensions to ignore.

alternatively, we could say: if it has >= 2 '.', skip it

[emaste]

alternatively, we could say: if it has >= 2 '.', skip it

Yes I think that's reasonable if you're up for updating this review with that change (or I'll take this one as is).

[freebsd_igalic.co]

In D27959#624597, @emaste wrote:

alternatively, we could say: if it has >= 2 '.', skip it

Yes I think that's reasonable if you're up for updating this review with that change (or I'll take this one as is).

naïvely, i'd implement this like,

char *first_dot  = strchr(p->fts_name, '.');
char *last_dot = strchrt(p->fts_name, '.');
if (first_dot != NULL && last_dot != NULL && first_dot != last_dot)
        continue;

assuming that .symbols and .debug files usually look like: foo.ko.debug and bar.ko.symbols like with .pkgsave files.

[freebsd_igalic.co]

same functionality, written in C instead of python style C. thanks to RhodiumToad:

char *dot = strchr(p->fts_name, '.');
if (dot && strchr(dot+1, '.') != NULL)
        continue;

i don't suppose dot+1 will push us *past* \0, so we don't need to check if we still have enough string left

[freebsd_igalic.co]

fix @emaste's formatting remarks from IRC

[kevans]

usr.sbin/kldxref/kldxref.c

762

I think we can safely drop .symbols from this list, @emaste transitioned to .debug in 05117b57a54a -- surely enough time has passed that we can stop explicitly caring about or referencing them, hopefully...

[freebsd_igalic.co]

address @kevans' feedback.

[freebsd_igalic.co]

address @kevans' feedback, by moving local_rc *.pkgsave skipping into the skipping of scratch files, since that issues a warning.

[freebsd_igalic.co]

unfucked this differential, after accidentally updating with the wrong diff

[freebsd_igalic.co]

I'm going to update this review with an edit to the man page, and then we can revive it.

[freebsd_igalic.co]

add documentation

[pauamma_gundo.com]

You didn't ask for manpages approval. Is that deliberate?

usr.sbin/kldxref/kldxref.8

55 ↗

(On Diff #117644)

at least two.

Also, is it acceptable to ignore "foo.bar.ko"?

[freebsd_igalic.co]

usr.sbin/kldxref/kldxref.8

55 ↗

(On Diff #117644)

true, we just stop counting after two.

There's currently no module named that way

[freebsd_igalic.co]

In D27959#880578, @pauamma wrote:

You didn't ask for manpages approval. Is that deliberate?

I thought requiring manpages is automatic as soon as you add/modify a man page

[freebsd_igalic.co]

address @pauamma's feedback.

[debdrup]

Please remember to bump .Dd :)

[pauamma_gundo.com]

In D27959#880825, @debdrup wrote:

Please remember to bump .Dd :)

Yeah. Whole text shows it already there for me, but in both old and new, so I suspect something.

[imp]

I think this is now missing the .c code changes.

[freebsd_igalic.co]

one day, i will use arc and it will not mess up my review.
one day, really soon.

[imp]

It's fine this way...
Or we could also just whine less with weird files we encounter, though that's likely riskier than this tactic.

usr.sbin/kldxref/kldxref.c

760

This should be

/*
 * Skip files that generate errors like .debug, .symbol and .pkgsave
 * by generally skipping all files with two dots.
 */

since it's a multi-line comment.

[freebsd_igalic.co]

address @imp's feedback.

[pauamma_gundo.com]

Minor nit fixable on commit.

usr.sbin/kldxref/kldxref.8

61 ↗

(On Diff #117725)

I'd either remove the comma or add another before "however".