On 21.11.25 14:32, Schwarz, Konrad (FT RPD CED OES-DE) wrote:
>> From: Kiszka, Jan (FT RPD CED) <[email protected]>
>> 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
