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] Make the compiler do the math.


Hi Nick,

Nick Clifton wrote:


* pe-dll.c (AFET): New macro.


The name "AFET" is a little bit obscure. I would have thought a more self-documenting name would be better, eg AUTOFILTER_ENTRY or STRING_AND_LEN.

Isn't A_uto_F_ilter_E_ntry_T_ype self documenting? :)
I chose a short name, because it shows up a lot in the file, and because it is private to the file, so you can't miss it.
But I can change it, no prop.


Also I think that it would be a great idea if this macro were placed in a header file (eg bfd.h via bfd-in.h) so that it could be used elsewhere in the binutils code (eg bfd/elf.c, bfd/archive.c, binutils/readelf.c). In this case I think that a more generic name for the macro would be appropriate (eg STRING_COMMA_LEN).


Ah, OK, better than my STR_STRLEN_PAIR. I'm not good at naming stuff.

I played a little with this idea, and came up with these extra macros.

#define const_strcmp(DST, ORG) \
   strncmp (DST, ORG "", sizeof (ORG) - 1)

#define STR_STRLEN_ASSERT(COND, MSG) \
   sizeof (struct { char MSG[ !(COND) ? -1 : 1 ]; })

#define check_strncmp(DST, SRC, N) \
   ( STR_STRLEN_ASSERT ((sizeof (SRC "") - 1) == (N), \
             check_strncmp_length_mismatch), \
   strncmp (DST, SRC, N) )

With these you can catch more errors at compile time, and they add nothing to
the binary even at -O0. Only at -g3 they generate more debug info, but that should
be no problem.
Other that struct initializers, strncmp is the biggest candidate for STRING_COMMA_LEN.
These macros obviate the need to litter the code with a macro that has a free comma in it.


Simple example use cases (found by grepping for strncmp):

bfd/cofflink.c:235
-       && !strncmp (name,"__imp_", 6))
+       && !const_strncmp (name,"__imp_"))

With const_strncmp you don't have to maintain the len by hand, you keep efficiency, and you have a compiler
error if you pass it a non compile time constant string.


bfd/archive.c:916
-    if (!strncmp (nextname, "__.SYMDEF       ", 16)
+   if (!check_strncmp (nextname, "__.SYMDEF       ", 16)

check_strncmp checks if the len of the src string matches the len passed in the third parameter.
For cases where the passed len is info that should be kept visible.
It checks that the len passes is the same of the len of right string passed.
It keeps the same efficiency, and you have a compiler error if you pass it a non compile time constant string.


Do you think this is valuable for binutils?

Maybe a better name for these macros would have 'prefix' in it?
  const_strprefixcmp ?
  check_strprefixcmp ?

Attached is a tar.gz with some compilable examples of this.

I can prepare a patch with these macros and with STRING_COMMA_LEN
if there is interest, or just with STRING_COMMA_LEN.

Of course I would fold STR_STRLEN_ASSERT into check_strncmp to
minimize the namespace clutter.

On top of that patch I will post revised versions of that patch agains pe-dll.c and elf.c that use the new global macros,
instead of private ones.


Cheers,
Pedro Alves

Attachment: strstrlen.tar.gz
Description: GNU Zip compressed data


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