Subject: Re: constants for tar header offsets

From: Dagfinn Ilmari Mannsåker <[email-masked]>
Date: Tue, 18 Apr 2023 17:56:43 +0100

Robert Haas <[email-masked]> writes:

> On Tue, Apr 18, 2023 at 12:06 PM Tom Lane <[email-masked]> wrote:
>> Hmm, you're right: I checked the POSIX.1-2018 spec as well, and
>> it agrees that the prefix field is 155 bytes long.  Perhaps just
>> add another comment line indicating that 12 bytes remain unassigned?
>
> OK. Here's v2, with that change and a few others.

It still has magic numbers for the sizes of the fields, should those
also be named constants?

- ilmari

---

From: Robert Haas <[email-masked]>
Date: Wed, 19 Apr 2023 09:09:52 -0400

> It still has magic numbers for the sizes of the fields, should those

I thought about that. It's arguable, but personally, I don't think
it's worth it. If the concern is greppability, having constants for
the offsets is good enough for that. If the concern is making it
error-free, I think we'd be well-advised to consider bigger redesigns
of the API. For example, we could have a function
read_number_from_tar_header(char *pointer_to_the_start_of_the_block,
enum which_field) and then that function could encapsulate the
knowledge of which tar numbers are 8 bytes and which are 12 bytes.
Writing read_number_from_tar_header(h, TAR_FIELD_CHECKSUM) seems
potentially less error-prone than
read_tar_number(&h[TAR_OFFSET_CHECKSUM], 8). On the other hand,
changing the latter to read_tar_number(&h[TAR_OFFSET_CHECKSUM],
TAR_LENGTH_CHECKSUM) seems longer but not necessarily cleaner. So I
felt it didn't make sense.

Just to be clear, I don't have a full vision for what a replacement
API ought to look like, and I'm not sure that figuring that out is
something that has to be done right this minute. I proposed this patch
not because it's perfect, but because it's simple. We can think of
doing more in the future if someone wants to devote the effort, and
that person might even be me, but right now it's not.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: "Tristan Partin" <[email-masked]>
Date: Tue, 01 Aug 2023 10:07:10 -0500

> that person might even be me, but right now it's not.

A new API design would be great, but for right now v2 is good enough and 
should be committed. It is much easier to read the code with this patch 
applied.

Marking as "Ready for Committer" since we all seem to agree that this is 
better than what exists.

-- 
Tristan Partin
Neon (https://neon.tech)

---

From: Robert Haas <[email-masked]>
Date: Tue, 1 Aug 2023 13:57:02 -0400

> Marking as "Ready for Committer" since we all seem to agree that this is

Thanks, committed now.