This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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: [Patch - hppa] add compile time check for immediate values of shift/deposit/extract


> The patch below adds compile time checks to gas which (where possible)
> will check the constant immediate values of the shift/deposit/extract
> functions and warn the user if he coded a dangerous combination.

This is a good idea.

>         * gas/config/tc-hppa.c: Add check of immediate values.

This can be improved.  Mention new define.  The function, pa_ip, where
the change is made should be added after the file name.

> +/* Store immediate values of shift/deposit/extract functions.  */
> +
> +#define INSERT_IMMEDIATE(VALUE) \

Suggest SAVE_IMMEDIATE would be a better name for macro.

> +  { \
> +    if (immediate_check) \
> +	{ if (sv1 == -1) sv1 = VALUE; \
> +	  else if (sv2 == -1) sv2 = VALUE; \

GNU style would place "if" on separate line from "{".  I would rename
sv1 and sv2 to pos and len to correspond to the terminology used in the
arch.  Hopefully, pos and len aren't already used.

> +  if (immediate_check) {

Paren should be on next line and indented two spaces.

> +    if (sv1 != -1 && sv2 != -1 && (sv2-1) > sv1)

The parens around "sv2-1" are unnecessary.  There should be one space before
and after "-".  Rewritten, this would look like:

	if (pos != -1 && len != -1 && pos < len - 1)

This matches the undefined test in the arch.

Thanks,
Dave
-- 
J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
National Research Council of Canada              (613) 990-0752 (FAX: 952-6602)


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