Re: [PATCH 2/6] gdb/testsuite/dwarf: convert _section proc to use parse_options

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2025-11-19 10:53, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> I meant to send this last week but forgot.
> 
> Simon> -    proc _section {name {flags ""} {type ""}} {
> Simon> -	if {$name == ".debug_str"} {
> Simon> -	    # Hard-code this because it's always desirable.
> Simon> -	    _emit "        .section $name, \"MS\", %progbits, 1"
> Simon> -	} elseif {$flags == "" && $type == ""} {
> Simon> -	    _emit "        .section $name"
> Simon> -	} elseif {$type == ""} {
> Simon> -	    _emit "        .section $name, \"$flags\""
> Simon> -	} else {
> Simon> -	    _emit "        .section $name, \"$flags\", %$type"
> 
> This code ignores flags and type if name is .debug_str, but the
> replacement code:
> 
> Simon> +	if { $name == ".debug_str" } {
> Simon> +	    # Hard-code this because it's always desirable.
> Simon> +	    append directive ", \"MS\", %progbits, 1"
> Simon> +	}
> Simon> +
> Simon> +	if { $flags != "" } {
> Simon> +	    append directive ",\"$flags\""
> Simon> +	}
> Simon> +
> Simon> +	if { $type != "" } {
> Simon> +	    # Can't specify a type without flags.
> Simon> + 	    if { $flags == "" } {
> Simon> +		append directive ", \"\""
> Simon> +	    }
> Simon> +
> Simon> +	    append directive ", %$type"
> Simon> +	}
> 
> ... does not.
> 
> I think the old code was clearer in this regard.  I didn't read forward
> in the series to see if this restructuring helps newer code though.
> Anyway this change should be addressed.

It's true that if one was to call the proc with $name == ".debug_str"
and some flags, the resulting assembler directive would be malformed.  I
can change it to

	if { $name == ".debug_str" } {
	    # Hard-code this because it's always desirable.
	    _emit "        .section .debug_str, \"MS\", %progbits, 1"
	    return
	}

I think that would make it clear again that this is a special case,
disconnected from the rest.

Or something cleaner would be to remove this bit from this proc, and
add a _section_debug_str proc that would call _section with the right
params (might need a _defer_output_debug_str as well).

Simon