Cast alignment warnings

[Julien ÉLIE]

Hi Russ,

Following up an old thread:

>> We have in timecaf/timehash an unsigned char class that is casted to an
>> unsigned int* for the sake of sscanf().
>> 
>>      n = sscanf(path, "timecaf-%02x/%02x/%04x.CF", (unsigned int*)&class, &t1, &t2);
> 
> Actually, I think that one *is* a problem; the address of class is not
> guaranteed to be aligned, since it's a char, which means that sscanf may
> do an unaligned store of an integer.  It's also wrong for other reasons:
> integers are probably either four or eight bytes, so sscanf is going to
> write at least four bytes at the address of class, which is going to
> blithely overwrite neighboring variables on the stack.
> 
> That looks like a real bug.  The code needs to declare a temporary
> unsigned int variable, sscanf into it, and then store the results in
> class.

So if I understand well, do we just need doing the following thing?
(It builds fine, without warning.)


--- storage/timecaf/timecaf.c	(révision 9831)
+++ storage/timecaf/timecaf.c	(copie de travail)
@@ -186,15 +186,16 @@
 
 static TOKEN *PathNumToToken(char *path, ARTNUM artnum) {
     int			n;
-    unsigned int        t1, t2;
+    unsigned int        tclass, t1, t2;
     STORAGECLASS        class;
     time_t              timestamp;
     static TOKEN	token;
 
-    n = sscanf(path, "timecaf-%02x/%02x/%04x.CF", (unsigned int *)&class, &t1, &t2);
+    n = sscanf(path, "timecaf-%02x/%02x/%04x.CF", &tclass, &t1, &t2);
     if (n != 3)
 	return (TOKEN *)NULL;
     timestamp = ((t1 << 8) & 0xff00) | ((t2 << 8) & 0xff0000) | ((t2 << 0) & 0xff);
+    class = tclass;
     token = MakeToken(timestamp, artnum, class, (TOKEN *)NULL);
     return &token;
 }



--- storage/timehash/timehash.c	(révision 9831)
+++ storage/timehash/timehash.c	(copie de travail)
@@ -114,16 +114,17 @@
 
 static TOKEN *PathToToken(char *path) {
     int			n;
-    unsigned int        t1, t2, t3, seqnum;
+    unsigned int        tclass, t1, t2, t3, seqnum;
     STORAGECLASS        class;
     time_t		now;
     static TOKEN	token;
 
     n = sscanf(path, "time-%02x/%02x/%02x/%04x-%04x",
-               (unsigned int *)&class, &t1, &t2, &seqnum, &t3);
+               &tclass, &t1, &t2, &seqnum, &t3);
     if (n != 5)
 	return (TOKEN *)NULL;
     now = ((t1 << 16) & 0xff0000) | ((t2 << 8) & 0xff00) | ((t3 << 16) & 0xff000000) | (t3 & 0xff);
+    class = tclass;
     token = MakeToken(now, seqnum, class, (TOKEN *)NULL);
     return &token;
 }



--- nnrpd/sasl.c	(révision 9831)
+++ nnrpd/sasl.c	(copie de travail)
@@ -115,6 +115,7 @@
     const char *mech;
     const char *clientin = NULL;
     unsigned int clientinlen = 0;
+    size_t tclientinlen = 0;
     const char *serverout = NULL;
     unsigned int serveroutlen;
     char base64[BASE64_BUF_SIZE+1];
@@ -209,7 +210,8 @@
 
 	/* Get the response from the client. */
 	r1 = line_read(&NNTPline, PERMaccessconf->clienttimeout,
-		      &clientin, (size_t *) &clientinlen, NULL);
+		      &clientin, &tclientinlen, NULL);
+        clientinlen = tclientinlen;
         
         switch (r1) {
 	case RTok:








I've just come across the following ones:

buffindexed/buffindexed.c:1051:20: warning: cast from 'char *' to 'GROUPENTRY *'
      increases required alignment from 1 to 8 [-Wcast-align]
    GROUPentries = (GROUPENTRY *)((char *)GROUPheader + sizeof(GROUPHEADER));
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buffindexed/buffindexed.c:1206:18: warning: cast from 'char *' to 'GROUPENTRY *'
      increases required alignment from 1 to 8 [-Wcast-align]
  GROUPentries = (GROUPENTRY *)((char *)GROUPheader + sizeof(GROUPHEADER));
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buffindexed/buffindexed.c:1241:18: warning: cast from 'char *' to 'GROUPENTRY *'
      increases required alignment from 1 to 8 [-Wcast-align]
  GROUPentries = (GROUPENTRY *)((char *)GROUPheader + sizeof(GROUPHEADER));
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
buffindexed/buffindexed.c:1667:15: warning: cast from 'char *' to 'OVBLOCK *'
      (aka 'struct _OVBLOCK *') increases required alignment from 1 to 8
      [-Wcast-align]
    ovblock = (OVBLOCK *)((char *)addr + pagefudge);
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Wouldn't it be enough for them to only add a cast to (void *) just before
we cast to (GROUPENTRY *) or (OVBLOCK *)?  The alignment should normally
be good here, because we already are pointing to an address within
the header or block.

-- 
Julien ÉLIE

« Vinum bonum laetificat cor hominis. »