[Linux-HA] stonith apcmastersnmp relocation error FIXED

[Andreas Huck]

Hi,

On Saturday 05 June 2004 14:44, Lars Ellenberg wrote:
> / 2004-06-05 13:45:06 +0200
>
> \ Andreas Piesk:
> > Andreas Huck schrieb:
> > >Hi,
> > >
> > >On Friday 04 June 2004 22:23, Andreas Piesk wrote:
> > >[...]
> > >
> > >>>    // issue a warning if ident mismatches
> > >>>-    if (strcmp(ident, TESTED_IDENT) != 0) {
> > >>>+    for(i=sizeof(APC_tested_ident)/sizeof(APC_tested_ident[0]) -1; i
> > >>> >=0 ; i--) +      if (!strcmp(ident, APC_tested_ident[i])) break; +  
> > >>>  if (i<0) {
> > >>>       syslog(LOG_WARNING,
> > >>>              "%s: module not tested with this hardware '%s'",
> > >>>              __FUNCTION__, ident);
> > >>
> > >>hmm, i don't like the for-loop especially the division. how about
> > >>something like that:
> > >>
> > >>static const char* APC_tested_ident[] = {"AP9606", "AP7920",
> > >>                                         "AP_other_well_tested",
> > >>                                          NULL};
> > >>int i=0;
> > >>while( APC_tested_ident[i] != NULL &&
> > >>       strcmp(ident,APC_tested_ident[i])) i++;
> > >>
> > >>  if( APC_tested_ident[i] == NULL ) {
> > >>    // not tested
> > >
> > >hm, you exchange a single integer division by a dependency (NULL has to
> > >be the last entry). But OK, your code is easier to read, go ahead.
> >
> > you are right about the dependency. this problem can easily be solved by
> > using a macro to build the APC_tested_ident array:
> >
> > #define IDENTS(b ... ) {b,NULL}
> > char *APC_tested_idents[]=IDENTS("AP9606","AP7920","AP1234");
> >
> > about the integer division: i'm not concerned about the division itself.
> > the method size of array/number of elements depends on the size of the
> > elements. what if apc changes the ident to something like 'APC
> > SUPERDUPER 0815'?
>
> nothing.
> APC_tested_ident is an array of char* .
> sizeof(APC_tested_ident) has nothing to do with the length of the
> cstrings those char* point to.
>  #define NR_ARRAY_ELEMENTS(A) (sizeof(A)/sizeof(A[0]))

in  linux-ha/portability.h, which is included in apcmastersnmp.c, we already have
#define 	DIMOF(a)                ((int) (sizeof(a)/sizeof(a[0])) )

so this makes the for-loop eaier to read:
+    for(i=DIMOF(APC_tested_ident) -1; i >=0 ; i--)
+      if (strcmp(ident, APC_tested_ident[i]) == 0) break;
+    if (i<0) {

Anyway.
Regards,
Andreas