Thread: use correct variable in error message in _allocAH function (pg_backup_archiver.c)

On 2025/04/15 7:21, Mahendra Singh Thalor wrote:
> Hi hackers,
> 
> In _allocAH function(pg_backup_archiver.c), we were using the *fmt* variable in switch case for *default case*, but
correctvariable is AH->format.
 
> 
> Here, I am attaching a patch for the same.

Thanks for the report and patch! It looks good to me.
Since this issue exists in the back branches,
the patch needs be back-patched to all supported versions.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

On 2025/04/15 17:02, Fujii Masao wrote:
> 
> 
> On 2025/04/15 7:21, Mahendra Singh Thalor wrote:
>> Hi hackers,
>>
>> In _allocAH function(pg_backup_archiver.c), we were using the *fmt* variable in switch case for *default case*, but
correctvariable is AH->format.
 
>>
>> Here, I am attaching a patch for the same.
> 
> Thanks for the report and patch! It looks good to me.
> Since this issue exists in the back branches,
> the patch needs be back-patched to all supported versions.

Unless there are any objections, I'll commit the patch with
the following commit message:

------------------
pg_dump: Fix incorrect archive format shown in error message.

In pg_dump and pg_restore, _allocAH() calls _discoverArchiveFormat() to
determine the archive format when the input format is unknown one.
If the input or discovered format is unrecognized, it reports an error
including the archive format number.

If discovered format is unrecognized, its number should be shown in
the error message. But previously the error message mistakenly showed
the originally requested format number (i.e., unknown one) instead of
the discovered one, due to referencing the wrong variable in the error
message.

This commit corrects the issue by using the appropriate variable in
the error message.

In practice, this has no visible effect for users, since
_discoverArchiveFormat() does not currently return unrecognized formats,
so the error condition is never actually triggered.

Back-patch to all supported versions.

Author: Mahendra Singh Thalor <mahi6run@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Discussion: https://postgr.es/m/CAKYtNAqu+N-Ab2Fq6wzNSOm_-0N-BMneanYNV1+6kFDXjva1Eg@mail.gmail.com
Backpatch-through: 13
------------------

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> Thanks for the report and patch! It looks good to me.

Agreed.

> Since this issue exists in the back branches,
> the patch needs be back-patched to all supported versions.

I doubt it's worth the trouble and buildfarm cycles to
back-patch, since this should be a can't-happen code path.
Worth fixing in HEAD, yes, but not convinced about doing
more than that.

            regards, tom lane

On 2025/04/15 23:37, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>> Thanks for the report and patch! It looks good to me.
> 
> Agreed.
> 
>> Since this issue exists in the back branches,
>> the patch needs be back-patched to all supported versions.
> 
> I doubt it's worth the trouble and buildfarm cycles to
> back-patch, since this should be a can't-happen code path.
> Worth fixing in HEAD, yes, but not convinced about doing
> more than that.

Yes, that makes sense. I'll apply the fix to the master branch only.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

On 2025/04/16 13:30, Fujii Masao wrote:
> 
> 
> On 2025/04/15 23:37, Tom Lane wrote:
>> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>>> Thanks for the report and patch! It looks good to me.
>>
>> Agreed.
>>
>>> Since this issue exists in the back branches,
>>> the patch needs be back-patched to all supported versions.
>>
>> I doubt it's worth the trouble and buildfarm cycles to
>> back-patch, since this should be a can't-happen code path.
>> Worth fixing in HEAD, yes, but not convinced about doing
>> more than that.
> 
> Yes, that makes sense. I'll apply the fix to the master branch only.

I've pushed the patch only to the master. Thanks!

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION