This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] mips-tdep.c: Update mips_register_to_value(), et al...


On 12/10/2010 08:34 AM, Kevin Buettner wrote:
>>>
>>> 	* mips-tdep.c (big_endian_4_byte_fp_reg_with_double_type_p)
>>> 	(eight_byte_gp_reg_with_shorter_type_p): New functions.
>>
>> Names of these two new functions are a little bit long.  :-)
> 
> I agree that they are long, but are they too long?
> 
> Here, taken from the patch, are the various uses of these functions:
> 
> mips_convert_register_p:
> +  return big_endian_4_byte_fp_reg_with_double_type_p (gdbarch, regnum, type)
> +      || eight_byte_gp_reg_with_shorter_type_p (gdbarch, regnum, type);
> 
> mips_register_to_value and mips_value_to_register:
> +  if (big_endian_4_byte_fp_reg_with_double_type_p (gdbarch, regnum, type))
> +  else if (eight_byte_gp_reg_with_shorter_type_p (gdbarch, regnum, type))
> 
> Note that the names are just (barely) short enough that the function
> parameters do not need to be placed on a separate line.
> 

Yes, from this point of view, they are not too long.

>> Sometimes, it is hard to describe function's behavior only by its name.
>>  Can we name them as mips_convert_register_p_{1,2}, and give comments in
>> details to each of them?
> 
> That could be done.  I would prefer to use names that are a bit more
> descriptive though.  I think that the names that I have chosen, as bad
> as they are, still make the code easier to understand than if generic
> predicate names were used.  I was hoping that someone would suggest some
> names that are shorter, but still reasonably descriptive.  But, as you
> say, that might not be possible.
> 
> Hmm...
> 
> Maybe it'd work out better if I put less detail in the name.  How
> about these names?
> 
>     mips_convert_register_float_case_p
>     mips_convert_register_gpreg_case_p
> 
> (Or something along those lines...)

Looks fine.

> 
> You'd still have to read the code, or possibly a comment, to find
> out the precise meaning of the predicate, but the names contain
> just enough information so that one won't unwittingly be confused
> with the other.

It is perfect to use a function according to the description of its
name, without reading comments and precise meaning of it.  Usually, it
might be impossible.  Even if we choose
"big_endian_4_byte_fp_reg_with_double_type_p" and
"eight_byte_gp_reg_with_shorter_type_p" for function names, some people
will still have to read the comments, at least, when they want to use
them.  Personally, I'd like to name function in a reasonably descriptive
way, and leave comments in details for that function.

-- 
Yao (éå)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]