Re: [PATCH 7/7] Prefer $NM @file over calculating the cmd line length.

[Ralf Wildenhues]

Hi Peter,

* Peter Rosin wrote on Sat, Sep 11, 2010 at 02:24:18PM CEST:
> Subject: [PATCH 7/7] Prefer $NM @file when the toolchain isn't native to 
> $build.
> 
> * libltdl/config/ltmain.m4sh (func_mode_link): Avoid calculating
> the command line length and take the @file branch *if* the file
> names needs to be converted for the toolchain and the @file
> branch works. Fixes stresstest.at when doing a cross from Cygwin

Two spaces after period.

> to MinGW using MinGW native tools (a.k.a. a "faked" cross).

> --- a/libltdl/config/ltmain.m4sh
> +++ b/libltdl/config/ltmain.m4sh
> @@ -7341,10 +7341,23 @@ EOF
>           save_ifs="$IFS"; IFS='~'
>           for cmd1 in $cmds; do
>             IFS="$save_ifs"
> -           eval cmd=\"$cmd1\"
> -           func_len " $cmd"
> -           len=$func_len_result
> -           if test "$len" -lt "$max_cmd_len" || test "$max_cmd_len" -le -1; 
> then
> +           # Take the normal branch if the nm_file_list_spec branch doesn't
> +           # work or if tool conversion is not needed

Period at end of sentence.

> +           case $nm_file_list_spec~$to_tool_file_cmd in
> +             *~func_convert_file_noop | *~func_convert_file_msys_to_w32 | ~*)
> +               tool_conversion_needed_and_working=false
> +               eval cmd=\"$cmd1\"
> +               func_len " $cmd"
> +               len=$func_len_result
> +               ;;
> +             *)
> +               tool_conversion_needed_and_working=:
> +               ;;
> +           esac
> +           if { test "$len" -lt "$max_cmd_len" \
> +                || test "$max_cmd_len" -le -1; } \
> +              && ! $tool_conversion_needed_and_working

`! $command' is not portable, please reverse the logic or use
test foo = bar or so.

The logic is flawed here, you are testing potentially-undefined $len.
Did you mean to reverse the && order (the shell does short-circuiting
of || and && so that would seem appropriate here)?

> +           then
>               func_show_eval "$cmd" 'exit $?'

This code has eval problems, but they were before your patch, so I guess
they are not on you to fix.

>               skipped_export=false
>             elif test -n "$nm_file_list_spec"; then

Thanks,
Ralf