Robert Haas <robertmhaas@gmail.com> writes:
> We have a few different places in the code where we generate or modify
> tar headers or just read data out of them. The code in question uses
> one of my less-favorite programming things: magic numbers. The offsets
> of the various fields within the tar header are just hard-coded in
> each relevant place in our code. I think we should clean that up, as
> in the attached patch.
Generally +1, with a couple of additional thoughts:
1. Is it worth inventing macros for the values of the file type,
rather than just writing the comment you did?
2. The header size is defined as 512 bytes, but this doesn't sum to 512:
+ TAR_OFFSET_PREFIX = 345 /* 155 byte string */
Either that field's length is really 167 bytes, or there's some other
field you didn't document. (It looks like you may have copied "155"
from an incorrect existing comment?)
> I hasten to emphasize that, while I think this is an improvement, I
> don't think the result is particularly awesome. Even with the patch,
> src/port/tar.c and src/include/pgtar.h do a poor job insulating
> callers from the details of the tar format. However, it's also not
> very clear to me how to fix that.
Yeah, this is adding greppability (a good thing!) but little more.
However, I'm not convinced it's worth doing more. It's not like
this data structure will change anytime soon.
regards, tom lane