Re: [PATCH 1/1] scripts: Changed potential O(n) file size calculation to O(1)

[Jan Kiszka <jan.kiszka@siemens.com>]

On 21.11.25 14:32, Schwarz, Konrad (FT RPD CED OES-DE) wrote:
>> From: Kiszka, Jan (FT RPD CED) <jan.kiszka@siemens.com>
>> Sent: Friday, November 21, 2025 11:16
>> Subject: Re: [PATCH 1/1] scripts: Changed potential O(n) file size calculation to
>> O(1)
>>  
>>>  scripts/mkemmc.sh | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/mkemmc.sh b/scripts/mkemmc.sh index
>>> 45dc3f08fa..d2c4e84b16 100755
>>> --- a/scripts/mkemmc.sh
>>> +++ b/scripts/mkemmc.sh
>>> @@ -37,13 +37,19 @@ usage() {
>>>      exit "$1"
>>>  }
>>>
>>> +file_size() {
>>> +	ls_line=$(ls -Hdog "$1") || return
>>
>> This will not suppress the error message when a file does not exist or is not
>> accessible, so:
>>
>> ls_line=$(ls -Hdog "$1" 2>/dev/null) || return
> 
> My take on this is:
> 
> `ls' is able to produce informative diagnostic messages as it has
> access to `errno'. The additional information, e.g., whether an 'EACCES',
> an `ENOENT' or an `ENOTDIR' error has occurred, should in the majority
> of cases help the user in fixing the underlying problem. I think it would be
> counter-productive to suppress this.

Even if that was true, you should not fold this change into your O(1)
optimization and argue about that separately.

> 
> (In fact, one could go further and consider using only the error message
> of `ls' and not provide an own error message at all.

I intentionally wanted to handle the error outside of the low-level
function here. The caller should not care how the size is retrieved -
implementation detail.

But the ls approach still has a major issue: It gives a size for a
directory, rather than failing with "not a readable file".

Jan

> In this case, it would be especially easy to return `ls' status back to 
> the script's invoker, by simply invoking `exit' without an argument.)
> 
>>
>>> +	printf %s\\n "$ls_line" | cut -d\  -f3
>>> +	unset ls_line
>>> +}
>>> +
>>>  process_size() {
>>>      name=$1
>>>      image_file=$2
>>>      alignment=$3
>>>      image_arg=$4
>>>      if [ "${image_arg#*:}" = "$image_arg"  ]; then
>>> -        if ! size=$(wc -c < "$image_file" 2>/dev/null); then
>>> +        if ! size=$(file_size "$image_file"); then
>>>              echo "Missing $name image '$image_file'." >&2
>>>              exit 1
>>>          fi
>>> @@ -105,7 +111,7 @@ check_truncation() {
>>>      if [ "$image_file" = "/dev/zero" ]; then
>>>          return
>>>      fi
>>> -    if ! actual_size=$(wc -c < "$image_file" 2>/dev/null); then
>>> +    if ! actual_size=$(file_size "$image_file"); then
>>>          echo "Missing image '$image_file'." >&2
>>>          exit 1
>>>      fi
>>
>> Thanks,
>> Jan
>>
>> --
>> Siemens AG, Foundational Technologies
>> Linux Expert Center

-- 
Siemens AG, Foundational Technologies
Linux Expert Center