[DTrace-devel] [PATCH 1/7] parser: check for all types of char

[Nick Alcock]

On 8 May 2023, Eugene Loh via DTrace-devel told this:

> I'm not 100% sure but:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
>
> Would be nice to have tests, but that might be impractical.

We do, but they are of the form "run with kernel newer than 6.3 and then
also with an older one", which is a bit hard to automate :)

> On 5/2/23 13:12, Nick Alcock via DTrace-devel wrote:
>> A while back I changed the check for IS_CHAR() in the parser to check
>> specifically for a char in the native signedness.  This fails now
>> the kernel might be using a char in the non-native signedness.
>
> s/fails/can fail/

Fixed.

> s/now the/now that the/

Both forms are grammatically correct :P (and to me it reads as clumsy to
put the 'that' in, as if I were a non-native speaker mentally
translating some other language to English. This may be a subtle
crosspondian language difference... I seem to recall reading that US
English is fonder of 'that' than British English is these days.)

>> Consider all such types to be equally-valid chars.  We really
>> don't care if they're signed or not, they're still chars.
>>
>> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
>> ---
>>   libdtrace/dt_parser.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libdtrace/dt_parser.h b/libdtrace/dt_parser.h
>> index a8aecb9a6ffef..1acbf4ed49b23 100644
>> --- a/libdtrace/dt_parser.h
>> +++ b/libdtrace/dt_parser.h
>> @@ -238,7 +238,7 @@ struct dtrace_typeinfo;	/* see <dtrace.h> */
>>   struct dt_pcb;		/* see <dt_impl.h> */
>>     #define	IS_CHAR(e) \
>> -	((((e).cte_format & CTF_CHAR) == CTF_CHAR) &&	\
>> +	((((e).cte_format & CTF_INT_CHAR) == CTF_INT_CHAR) && \
>>   	(e).cte_bits == NBBY)
>
> Since we're going from CTF_CHAR (potentially two nonzero bits) to CTF_INT_CHAR (a single nonzero bit), it suffices to say
> (cte_format & CTF_INT_CHAR)
> instead of
> ((cte_format & CTF_INT_CHAR) == CTF_INT_CHAR)
> but maybe the latter form is better for maintenance purposes???

Your form is definitely better. Adjusted.

-- 
NULL && (void)