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 Thu, 09 Dec 2010 17:01:24 +0800
Yao Qi <yao@codesourcery.com> wrote:

> On 12/09/2010 07:46 AM, Kevin Buettner wrote:
> > (Can anyone think of better names for the two new functions that I
> > introduced?)
> > 
> > 	* 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.

> 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...)

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.

Kevin


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