X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f X-Recipient: djgpp-workers AT delorie DOT com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type:content-transfer-encoding; bh=6nSfEPT7URRv/PPfQh9LzISD3G4cnGoRwlKvpe5g86k=; b=SE3bI/Tj4YCuh6/DUuUc4OeaaVp0+fffovQ+WZGHxRPxpZhRoU4y9vz89TEtJPvnfh w3YeEwwbNvYaZlvh47UUaTQtAFgQkbB0GLuKKHzWT4DzDdU7bTGNhV/6SyCX5UcgjzM7 1xRQyBE+ADFCEhrwGQTgJvi2/B0VCGFmCfjJI= MIME-Version: 1.0 In-Reply-To: <201106302109.30225.juan.guerrero@gmx.de> References: <201106302109 DOT 30225 DOT juan DOT guerrero AT gmx DOT de> Date: Thu, 30 Jun 2011 22:23:10 +0300 Message-ID: Subject: Re: [patch] fix epunzip stric aliasing warnings From: Ozkan Sezer To: djgpp-workers AT delorie DOT com Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by delorie.com id p5UJNEul003555 Reply-To: djgpp-workers AT delorie DOT com Errors-To: nobody AT delorie DOT com X-Mailing-List: djgpp-workers AT delorie DOT com X-Unsubscribes-To: listserv AT delorie DOT com Precedence: bulk On Thu, Jun 30, 2011 at 10:09 PM, Juan Manuel Guerrero wrote: > Am Dienstag, 28. Juni 2011 schrieb Ozkan Sezer: >> Hi: >> >> The attached patch fixes strict aliasing violation warnings in >> djtar/epunzip.c.  The warnings are only visible when using gcc<=3.3 >> (old but one I do use) but worth fixing, IMO, considering the shape >> of the code. >> >> Regards. >> >> -- >> O.S. >> > > I tested the patch submitted by Ozkan Sezer using gcc453b and gcc460b both > djdev204 versions and it fixes the strict warning issues.  The proposed changes > look ok to me.  If no one objects in a reasonable period of time I will commit > the patch below. > Just a note: Replacing (int *) casts onto char buffers by (__dj_int_a *) is not necessary, as char* is already a may_alias type. > Regards, > Juan M. Guerrero > > > >        * src/utils/djtar/epunzip.c (epunzip_read): Fix strict aaliasing warning. >        From Ozkan Sezer > > > > Index: djgpp/src/utils/djtar/epunzip.c > =================================================================== > RCS file: /cvs/djgpp/djgpp/src/utils/djtar/epunzip.c,v > retrieving revision 1.8 > diff -U 3 -r1.8 epunzip.c > --- djgpp/src/utils/djtar/epunzip.c     9 Jan 2011 14:19:59 -0000       1.8 > +++ djgpp/src/utils/djtar/epunzip.c     30 Jun 2011 19:08:00 -0000 > @@ -54,30 +54,30 @@ >    * for a one-segment ZIP, but fail on other headers. */ >   while(1) >     { > -      int buffer = 0; > +      char buffer[4]; > > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > > -      if(*(__dj_short_a *)&buffer != *(const short *)(const void *)"PK") > +      if(*(__dj_short_a *)buffer != ('K' << 8) + 'P') >        break; > > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > > -      if(*(__dj_short_a *)&buffer == *(const short *)(const void *)"\x3\x4") > +      if(*(__dj_short_a *)buffer == ('\004' << 8) + '\003') >        { >          /* local header */ >          at_local_header = 1; >          break; >        } > -      else if(*(__dj_short_a *)&buffer == *(const short *)(const void *)"\x30\x30") > +      else if(*(__dj_short_a *)buffer == ('0' << 8) + '0') >        { >          /* spanning marker, but only one segment >           * => need to find local header. */ >          continue; >        } > -      else if(*(__dj_short_a *)&buffer == *(const short *)(const void *)"\x7\x8") > +      else if(*(__dj_short_a *)buffer == ('\010' << 8) + '\007') >        { >          /* spanning marker, multiple segments. */ >          fprintf(log_out, "%s: spanning is not supported\n", zipfilename); > @@ -89,7 +89,7 @@ >          /* unknown header */ >          fprintf(log_out, >                  "%s: don't understand header type 0x%02x%02x\n", > -                 zipfilename, ((char *)&buffer)[0], ((char *)&buffer)[1]); > +                 zipfilename, buffer[0], buffer[1]); >          dont_understand = 1; >          break; >        } > @@ -100,25 +100,26 @@ > >   if (at_local_header) while(1) >     { > -      int buffer, ext_header, timedate, crc, size, length, name_length, > +      int ext_header, timedate, crc, size, length, name_length, >        extra_length, should_be_written, count, real_file = 0; > +      char buffer[4]; >       char filename[2048]; > >       if (!at_local_header) >        { > -         ((char *)&buffer)[0] = (char)get_byte(); > -         ((char *)&buffer)[1] = (char)get_byte(); > +         buffer[0] = (char)get_byte(); > +         buffer[1] = (char)get_byte(); > > -         if(*(__dj_short_a *)&buffer != *(const short *)(const void *)"PK") > +         if(*(__dj_short_a *)buffer != ('K' << 8) + 'P') >            { >              fprintf(log_out, "%s: invalid zip file structure\n", zipfilename); >              break; >            } > > -         ((char *)&buffer)[0] = (char)get_byte(); > -         ((char *)&buffer)[1] = (char)get_byte(); > +         buffer[0] = (char)get_byte(); > +         buffer[1] = (char)get_byte(); > > -         if(*(__dj_short_a *)&buffer != *(const short *)(const void *)"\x3\x4") > +         if(*(__dj_short_a *)buffer != ('\004' << 8) + '\003') >            { >              /* not a local header - all done */ >              break; > @@ -133,57 +134,57 @@ >       get_byte(); >       get_byte(); > > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > > -      if(*(__dj_short_a *)&buffer & CRYFLG) > +      if(*(__dj_short_a *)buffer & CRYFLG) >        { >          fprintf(log_out, "%s has encrypted file(s) - use unzip\n", zipfilename); >          break; >        } > -      ext_header = *(__dj_short_a *)&buffer & EXTFLG ? 1 : 0; > +      ext_header = *(__dj_short_a *)buffer & EXTFLG ? 1 : 0; > > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > > -      method = *(__dj_short_a *)&buffer; > +      method = *(__dj_short_a *)buffer; >       if(method != 8 && method != 0) >        { >          fprintf(log_out, "%s has file(s) compressed with unsupported method - use unzip\n", zipfilename); >          break; >        } > > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > -      ((char *)&buffer)[2] = (char)get_byte(); > -      ((char *)&buffer)[3] = (char)get_byte(); > -      timedate = buffer; > - > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > -      ((char *)&buffer)[2] = (char)get_byte(); > -      ((char *)&buffer)[3] = (char)get_byte(); > -      crc = buffer; > - > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > -      ((char *)&buffer)[2] = (char)get_byte(); > -      ((char *)&buffer)[3] = (char)get_byte(); > -      size = buffer; > - > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > -      ((char *)&buffer)[2] = (char)get_byte(); > -      ((char *)&buffer)[3] = (char)get_byte(); > -      length = buffer; > - > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > -      name_length = *(__dj_short_a *)&buffer; > - > -      ((char *)&buffer)[0] = (char)get_byte(); > -      ((char *)&buffer)[1] = (char)get_byte(); > -      extra_length = *(__dj_short_a *)&buffer; > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > +      buffer[2] = (char)get_byte(); > +      buffer[3] = (char)get_byte(); > +      timedate = *(__dj_int_a *)buffer; > + > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > +      buffer[2] = (char)get_byte(); > +      buffer[3] = (char)get_byte(); > +      crc = *(__dj_int_a *)buffer; > + > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > +      buffer[2] = (char)get_byte(); > +      buffer[3] = (char)get_byte(); > +      size = *(__dj_int_a *)buffer; > + > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > +      buffer[2] = (char)get_byte(); > +      buffer[3] = (char)get_byte(); > +      length = *(__dj_int_a *)buffer; > + > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > +      name_length = *(__dj_short_a *)buffer; > + > +      buffer[0] = (char)get_byte(); > +      buffer[1] = (char)get_byte(); > +      extra_length = *(__dj_short_a *)buffer; > >       for(count = 0; count < name_length; count++) >        { > @@ -277,34 +278,34 @@ > >          if(ext_header) >            { > -             ((char *)&buffer)[0] = (char)get_byte(); > -             ((char *)&buffer)[1] = (char)get_byte(); > -             ((char *)&buffer)[2] = (char)get_byte(); > -             ((char *)&buffer)[3] = (char)get_byte(); > -             if(buffer != *(int *)"PK\x7\x8") > +             buffer[0] = (char)get_byte(); > +             buffer[1] = (char)get_byte(); > +             buffer[2] = (char)get_byte(); > +             buffer[3] = (char)get_byte(); > +             if(*(__dj_int_a *)buffer != ('\010' << 24) + ('\007' << 16) + ('K' << 8) + 'P') >                { >                  fprintf(log_out, "%s: invalid zip file structure\n", >                          zipfilename); >                  break; >                } > > -             ((char *)&buffer)[0] = (char)get_byte(); > -             ((char *)&buffer)[1] = (char)get_byte(); > -             ((char *)&buffer)[2] = (char)get_byte(); > -             ((char *)&buffer)[3] = (char)get_byte(); > -             crc = buffer; > - > -             ((char *)&buffer)[0] = (char)get_byte(); > -             ((char *)&buffer)[1] = (char)get_byte(); > -             ((char *)&buffer)[2] = (char)get_byte(); > -             ((char *)&buffer)[3] = (char)get_byte(); > -             size = buffer; > - > -             ((char *)&buffer)[0] = (char)get_byte(); > -             ((char *)&buffer)[1] = (char)get_byte(); > -             ((char *)&buffer)[2] = (char)get_byte(); > -             ((char *)&buffer)[3] = (char)get_byte(); > -             length = buffer; > +             buffer[0] = (char)get_byte(); > +             buffer[1] = (char)get_byte(); > +             buffer[2] = (char)get_byte(); > +             buffer[3] = (char)get_byte(); > +             crc = *(__dj_int_a *)buffer; > + > +             buffer[0] = (char)get_byte(); > +             buffer[1] = (char)get_byte(); > +             buffer[2] = (char)get_byte(); > +             buffer[3] = (char)get_byte(); > +             size = *(__dj_int_a *)buffer; > + > +             buffer[0] = (char)get_byte(); > +             buffer[1] = (char)get_byte(); > +             buffer[2] = (char)get_byte(); > +             buffer[3] = (char)get_byte(); > +             length = *(__dj_int_a *)buffer; >            } > >        } > > Regards. -- O.S.