On Fri Feb 6, 2026 at 11:24 AM CET, Fiona Ebner wrote:
> Am 04.02.26 um 10:17 AM schrieb Daniel Kral:
>> Otherwise, if the underlying detect_architecture(...) method returns any
>> false value, the return value of the call to protected_call(...) will
>
> Do you mean undef value here? If I return 0 inside a protected call I get 0
>
>> return an empty string.
>
> not an empty string.
>
> There seems to be a difference in behavior between being in a nested
> protected call, which will return the result from the $sub directly, and
> a non-nested protected call, which reads the result from the pipe, which
> also results in an empty string when the result from $sub is undef.

Good catch, thanks! I only tried it with non-nested protected calls
which use the output from the pipe and also assumed that the empty
string comes from the conversion from a falsy value to a string in perl,
which is the empty string.

>
> I think the change here is fine, but we might want to document the
> behavior for protected call. Or fix up the non-nested case to properly
> return undef if $sub returns undef. Can be it's own series and will
> require checking that use sites are happy with the change, but it seems
> like there's only a small bunch that look at the return value anyways.

Right, as of writing this only get_os_release(), get_ct_init_path(), and
new() use the return value and all of these expect a string value.

For this series, I'll adapt the commit message in a v2.

>
>> This sets the architecture to an empty string and will make the
>> container fail to start.
>> 
>> Signed-off-by: Daniel Kral <[email protected]>
>> ---
>>  src/PVE/LXC/Setup.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/LXC/Setup.pm b/src/PVE/LXC/Setup.pm
>> index 113093d..fb0207e 100644
>> --- a/src/PVE/LXC/Setup.pm
>> +++ b/src/PVE/LXC/Setup.pm
>> @@ -153,7 +153,7 @@ sub new {
>>              warn "Architecture detection failed: $err" if $err;
>>          }
>>  
>> -        if (!defined($arch)) {
>> +        if (!$arch) {
>>              $arch = 'amd64';
>>              print "Falling back to $arch.\nUse `pct set VMID --arch ARCH` 
>> to change.\n";
>>          } else {




Reply via email to