dumprestore: extern the instance of union u_spcl
AbandonedPublic
Actions

Authored by mckusick on Mar 28 2020, 3:23 PM.

Details

Reviewers

Summary

When dumprestore.h is used in an application, it is intended that all compilation units share the single u_spcl instance declared. Mark it extern and define it in the two applications in base that use it.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 30199

Event Timeline

kevans created this revision.Mar 28 2020, 3:23 PM

Herald added a subscriber: imp. · View Herald TranscriptMar 28 2020, 3:23 PM

Harbormaster completed remote builds in B30148: Diff 69948.Mar 28 2020, 3:23 PM

kevans mentioned this in D24210: dump/restore: define variables in exactly one compilation unit.Mar 28 2020, 3:29 PM

While u_spcl is used only in dump/tape.c, it is used in four different files in restore and is not meant to be a separate instance in each one. So, you will have to make it an extern in the header file and then declare it in dump/tape.c and restore/dump.c. I prefer that you do it that way as opposed to the much uglier EXTERN approach.

This revision now requires changes to proceed.Mar 30 2020, 10:40 PM

kevans updated this revision to Diff 70040.Mar 31 2020, 12:36 AM

kevans retitled this revision from dumprestore: static'ify the instance of union u_spcl to dumprestore: extern the instance of union u_spcl.

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

Harbormaster completed remote builds in B30199: Diff 70040.Mar 31 2020, 12:36 AM

In D24209#533175, @mckusick wrote:

While u_spcl is used only in dump/tape.c, it is used in four different files in restore and is not meant to be a separate instance in each one. So, you will have to make it an extern in the header file and then declare it in dump/tape.c and restore/dump.c. I prefer that you do it that way as opposed to the much uglier EXTERN approach.

Done! Admittedly, I'm not a fan of the EXTERN pattern- it feels incredibly kludgy, but with most of the places I've ended up doing this it's because they don't have a good maintainer to consult on proper places to actually add the proper definitions -- with exception to dump/restore, which do have a good maintainer to consult but I'm unsure that you necessarily have the time to do so.

I do intend to make another pass later and eliminate the EXTERN in favor of hunting down the proper files to place definitions in everywhere that I've done this.

Now looks good.

I'll put my suggestion on how to divide up global variables in D24210. As noted there, I am willing to commandeer that phab task.

This revision is now accepted and ready to land.Mar 31 2020, 9:32 PM

These changes will be done as part of D24210 so that all the updates to make the dump and restore utilities compile with -fno-common are in one place.

This revision now requires review to proceed.Apr 3 2020, 11:50 PM

Changes moved to D24210.