#3648 PR `merged`: 'unset' local nested helper functions¶

Labels: cleanup, fixed / solved / done

jsmeix opened issue at 2026-10-02 11:43:¶

Type: Cleanup
Impact: Low
Reference to related issue (URL):

https://github.com/rear/rear/pull/3647#issuecomment-5951434875

How was this pull request tested?

"All checks have passed"

Description of the changes in this pull request:

Local nested helper functions must be 'unset'
because bash does not support 'local function ...'

codacy-production commented at 2026-10-02 11:44:¶

Up to standards ✅¶

🟢 Issues 0 issues

>
> > **Results:** > `0` new issues > > [View in > Codacy](https://app.codacy.com/gh/rear/rear/pull-requests/3648/issues) >

_{NEW Get contextual insights on your PRs based on Codacy's
metrics, along with PR and Jira context, without leaving GitHub. Enable
AI
reviewer}
_{TIP This summary will be updated as you push new changes.}

jsmeix commented at 2026-10-02 12:56:¶

I checked that each of the nested helper functions
is only called within its parent function where
the helper function is defined:

localhost:~/rear.github.master # for f in \
 add_replacement has_replacement get_replacement \
 syslinux_menu syslinux_menu_help \
 create_grub2_serial_entry create_grub2_rear_boot_entry \
 create_grub2_boot_next_entry create_grub2_reboot_entry \
 create_grub2_exit_entry ; \
do echo -n "$f : " ; \
   find usr/sbin/rear usr/share/rear/ -type f | xargs grep -l "$f" ; \
done

add_replacement : usr/share/rear/lib/layout-functions.sh
has_replacement : usr/share/rear/lib/layout-functions.sh
get_replacement : usr/share/rear/lib/layout-functions.sh
syslinux_menu : usr/share/rear/lib/bootloader-functions.sh
syslinux_menu_help : usr/share/rear/lib/bootloader-functions.sh
create_grub2_serial_entry : usr/share/rear/lib/bootloader-functions.sh
create_grub2_rear_boot_entry : usr/share/rear/lib/bootloader-functions.sh
create_grub2_boot_next_entry : usr/share/rear/lib/bootloader-functions.sh
create_grub2_reboot_entry : usr/share/rear/lib/bootloader-functions.sh
create_grub2_exit_entry : usr/share/rear/lib/bootloader-functions.sh

So

add_replacement has_replacement get_replacement

appear only in usr/share/rear/lib/layout-functions.sh
and

syslinux_menu syslinux_menu_help
create_grub2_serial_entry create_grub2_rear_boot_entry
create_grub2_boot_next_entry create_grub2_reboot_entry
create_grub2_exit_entry

appear only in usr/share/rear/lib/bootloader-functions.sh

Check that add_replacement has_replacement get_replacement
do not appear in usr/share/rear/lib/layout-functions.sh
outside of the function apply_layout_mappings()

localhost:~/rear.github.master # for f in \
 add_replacement has_replacement get_replacement ; \
do cat usr/share/rear/lib/layout-functions.sh \
   | sed -n -e '/^function apply_layout_mappings() {$/,/^}$/!p' \
   | grep $f ; \
done

[no output]

Check that syslinux_menu syslinux_menu_help
do not appear in usr/share/rear/lib/bootloader-functions.sh
outside of the function make_syslinux_config

localhost:~/rear.github.master # for f in \
 syslinux_menu syslinux_menu_help ; \
 do cat usr/share/rear/lib/bootloader-functions.sh \
    | sed -n -e '/^function make_syslinux_config {$/,/^}$/!p' \
    | grep $f ; \
done

[no output]

Check that

create_grub2_serial_entry create_grub2_rear_boot_entry
create_grub2_boot_next_entry create_grub2_reboot_entry
create_grub2_exit_entry

do not appear in usr/share/rear/lib/bootloader-functions.sh
outside of the function create_grub2_cfg

localhost:~/rear.github.master # for f in \
 create_grub2_serial_entry create_grub2_rear_boot_entry \
 create_grub2_boot_next_entry create_grub2_reboot_entry \
 create_grub2_exit_entry ; \
do cat usr/share/rear/lib/bootloader-functions.sh \
   | sed -n -e '/^function create_grub2_cfg {$/,/^# End of function create_grub2_cfg$/!p' \
   | grep $f ; \
done

[no output]

jsmeix commented at 2026-10-02 13:13:¶

I checked that the parent functions

apply_layout_mappings make_syslinux_config create_grub2_cfg

do not return before its nested helper functions are unset:

The functions make_syslinux_config create_grub2_cfg
do not call return:

localhost:~/rear.github.master # cat usr/share/rear/lib/bootloader-functions.sh \
| sed -n -e '/^function make_syslinux_config {$/,/^}$/p' \
| grep return

[no output]

localhost:~/rear.github.master # cat usr/share/rear/lib/bootloader-functions.sh \
| sed -n -e '/^function create_grub2_cfg {$/,/^# End of function create_grub2_cfg$/p' \
| grep return

[no output]

The function apply_layout_mappings calls return
(excerpts):

    is_true "$MIGRATION_MODE" || return 0
...
    test -s "$file_to_migrate" || return 0
...
    is_completely_identical_layout_mapping && return 0
...
    function add_replacement() {
...
    function has_replacement() {
...
    function get_replacement() {
        local item replacement junk
        read item replacement junk < <( grep "^$1 " $replacement_file )
        test "$replacement" && echo "$replacement" || return 1
    }
...
    done < "$replacement_file"
    # It is the responsibility of the caller of this apply_layout_mappings function what to do when it failed
    # (e.g. error out, retry, show a user dialog, or whatever is appropriate in the caller's environment):
    is_true $apply_layout_mappings_succeeded && return 0 || return 1
}

this is the state before the changes in this pull request
so inserting the unset before the final return like

    done < "$replacement_file"

    unset -f add_replacement has_replacement get_replacement

    # It is the responsibility of the caller ...
    # (e.g. error out, ...
    is_true $apply_layout_mappings_succeeded && return 0 || return 1
}

as done in this pull request is the correct place.

jsmeix commented at 2026-10-06 13:51:¶

Because "All checks have passed"
I think it works sufficiently well.

pcahyna commented at 2026-10-06 14:01:¶

@jsmeix looking ...

jsmeix commented at 2026-10-06 14:36:¶

@pcahyna
thank you for your suggestions!

I will update
https://github.com/rear/rear/wiki/Coding-Style#functions

Regarding
"functions in a script that are not used beyond the script":
I think finding such functions could be a good task for AI.
As time permits (looks hopeless with never ending CVEs)
I may try to get that done.

Next generic thing for AI are local variables in scripts
that are not yet defined as 'local'.

A related generic thing for AI are global variables
with names that are not uppercase.

pcahyna commented at 2026-10-06 14:46:¶

Regarding
"functions in a script that are not used beyond the script":
I think finding such functions could be a good task for AI.

I did not mean that we need to find them and fix now, but rather that a guideline in Coding Style should encompass that case (if you are going to write one).

pcahyna commented at 2026-10-06 14:51:¶

... I think finding such functions could be a good task for AI.

... Next generic thing for AI are local variables in scripts
that are not yet defined as 'local'.

I believe that such mechanical tasks are better handled by traditional rule-based automata (cf. your sed / grep pipelines that you used to verify the results above) and a good task for (LLM-based) AI is to write such rules (shell commands or otherwise). Perhaps that's what you meant in fact. An AI coding assistant will likely anyway handle the task by constructing shell pipelines of this type and running them.

(looks hopeless with never ending CVEs)

same here...

jsmeix commented at 2026-10-20 15:05:¶

I enhanced
https://github.com/rear/rear/wiki/Coding-Style#functions
and removed the comments in the code here via my two recent commits.

jsmeix commented at 2026-10-20 15:06:¶

The function bios_grub_install() is only called within
usr/share/rear/finalize/Linux-i386/660_install_grub2.sh
so it should be unset in that script.

jsmeix commented at 2026-10-20 15:44:¶

https://github.com/rear/rear/pull/3648/commits/e67fa0640a5bb7a2f686f53a0cdee71f4889e29c
shows that it can become tricky to properly
unset locally meant helper functions because
unset must happen before each return
after the helper function definition
so it helps to have helper function definitions
as late as possible.

pcahyna commented at 2026-10-20 18:12:¶

e67fa06 shows that it can become tricky to properly unset locally meant helper functions because unset must happen before each return after the helper function definition so it helps to have helper function definitions as late as possible.

Oops. It could then be preferable to not use nested functions and cope with them being global, as this looks difficult to keep track of.

jsmeix commented at 2026-10-21 10:40:¶

The errors in the log of the current failed check
https://download.copr.fedorainfracloud.org/results/packit/rear-rear-3648/fedora-rawhide-x86_64/11178011-rear/builder-live.log.gz
seem to not belong to ReaR
but to setting up the fedora-rawhide-x86_64 system

pcahyna commented at 2026-10-21 10:51:¶

The errors in the log of the current failed check https://download.copr.fedorainfracloud.org/results/packit/rear-rear-3648/fedora-rawhide-x86_64/11178011-rear/builder-live.log.gz seem to not belong to ReaR but to setting up the fedora-rawhide-x86_64 system

Indeed. I am looking into it.

jsmeix commented at 2026-10-21 10:57:¶

Despite that it can become tricky
to properly unset local functions
I would prefer to use local functions
when a function is not sufficiently implemented
so that it can be safely used globally / everywhere
to avoid that locally intended specific helper functions
could be misused as generic global functions, see
https://github.com/rear/rear/wiki/Coding-Style#functions

When a local function is not unset in some cases
nothing goes directly wrong because of this.
It only results that in some cases this local function
could be misused as a generic global function.
As long as local functions get unset at the end of the
intended scope (end of parent function / end of script)
things should be usually "good enough" and missing
unset calls can be easily added later as needed.

jsmeix commented at 2026-10-21 10:58:¶

@rear/contributors
I would like to merge it on Friday (23. Oct.) afternoon
unless there are objections.

[Export of Github issue for rear/rear.]

#3648 PR merged: 'unset' local nested helper functions¶