Re: Is it correct to say, "invalid data in file \"%s\"", BACKUP_LABEL_FILE in do_pg_backup_stop?

At Wed, 20 Jul 2022 17:09:09 +0530, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote in
> Hi,
>
> After the commit [1], is it correct to say errmsg("invalid data in file
> \"%s\"", BACKUP_LABEL_FILE))); in do_pg_backup_stop() when we hold the
> contents in backend global memory, not actually reading from backup_label
> file? However, it is correct to say that in read_backup_label.
>
> IMO, we can either say "invalid backup_label contents found" or we can be
> more descriptive and say "invalid "START WAL LOCATION" line found in
> backup_label content" and "invalid "BACKUP FROM" line found in
> backup_label content" and so on.
>
> Thoughts?

Previously there the case the "char *labelfile" is loaded from a file,
but currently it is alwasy a string build on the process. In that
sense, nowadays it is a kind of internal error, which I think is not
supposed to be exposed to users.

So I think we can leave the code alone to avoid back-patching
obstacles. But if we decided to change the code around, I'd like to
change the string into a C struct, so that we don't need to parse it.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center