Page MenuHomeFreeBSD

bspatch: use sizeof(header) rather than hardcoded size
ClosedPublic

Authored by emaste on Sep 12 2016, 2:53 PM.

Details

Summary

Although the header size is fixed, this change provides an explicit indication of what the size is being used for.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

emaste updated this revision to Diff 20276.Sep 12 2016, 2:53 PM
emaste retitled this revision from to bspatch: use sizeof(header) rather than hardcoded size.
emaste updated this object.
emaste edited the test plan for this revision. (Show Details)
emaste added reviewers: cperciva, allanjude, jhb.
allanjude accepted this revision.Sep 12 2016, 3:04 PM
allanjude edited edge metadata.

+1 for killing magic numbers

This revision is now accepted and ready to land.Sep 12 2016, 3:04 PM
emaste updated this revision to Diff 20315.Sep 13 2016, 7:52 PM
emaste edited edge metadata.
emaste added a subscriber: kib.
  • update with _Static_assert and use %jd / intmax_t as suggested by @kib
  • add an offset variable that we increment by the header / bzctrllen / bzdatalen to avoid repeating the same expression
This revision now requires review to proceed.Sep 13 2016, 7:52 PM
kib added a comment.Sep 14 2016, 10:30 AM

I looked at this stuff again, and having the construct char header[32] and static_assert(sizeof(header) == 32) feels weird. Might be, instead use a #define HEADER_SIZE 32 and then char header[HEADER_SIZE]; and use HEADER_SIZE.

Sorry for bike-shedding, but this is weird when seen completed.

emaste updated this revision to Diff 20341.Sep 14 2016, 1:02 PM
emaste edited edge metadata.

Incorporate comments from @kib

kib accepted this revision.Sep 14 2016, 1:18 PM
kib added a reviewer: kib.
This revision is now accepted and ready to land.Sep 14 2016, 1:18 PM

Commit summary will be use #define for header size instead of magic number Also note that the update includes a change to keep an increasing offset rather than repeating the same expression in several places.

allanjude accepted this revision.Sep 14 2016, 8:50 PM
allanjude edited edge metadata.

goatforit

This revision was automatically updated to reflect the committed changes.