Re: gcc-4.1.2 miscompiles setedit from CVS

[Witold F. <wi...@po...>]

On Wed, Aug 23, 2006 at 09:40:50PM +0200, Baurzhan Ismagulov wrote:
> On Wed, Aug 23, 2006 at 01:36:50PM -0300, Salvador Eduardo Tropea wrote:
> > >The second error is related to packed attribute in setedit/runprog.cc:
> > >
> > >g++  -I../include -I../setedit/include -I../settvuti/include
> > >-I../sdg/include -I../infview/include -I../../tvision/include -I../extra
> > >-I../easydiag -I../librhuti -I../mp3 -I../calcu -I../holidays
> > >-DFOR_EDITOR -O2 -pipe  -c ../setedit/runprog.cc -o obj/runprog.o
> > >../setedit/runprog.cc: In function 'void ConfigureRunCommand()':
> > >../setedit/runprog.cc:95: warning: 'packed' attribute ignored for field
> > >of type 'char [256]'
> > >../setedit/runprog.cc:99: warning: 'packed' attribute ignored for field
> > >of type 'char [4]'
> > >../setedit/runprog.cc:116: error: cannot bind packed field
> > >'box.ConfigureRunCommand()::<anonymous
> > >struct>::tl.TListBoxRec::selection' to 'ccIndex&'
> > 
> > I don't understand it. Sure that char[4] is already 32 bits aligned but 
> > I don't want to assume that it means that this is also "aligned" for 64 
> > and 128 bits machines. So that's a silly warning.
> 
> The warning is not silly, it says that the compiler ignores the
> attribute. __attribute__((packed)) packs the field itself (which in case
> of char[] is already packed), it doesn't have any impact on the
> surrounding fields. You probably want the following (untested):
> 
> diff -Naurp setedit.orig/setedit/runprog.cc setedit/setedit/runprog.cc
> --- setedit.orig/setedit/runprog.cc	2004-08-11 19:34:06.000000000 +0200
> +++ setedit/setedit/runprog.cc	2006-08-23 21:28:19.000000000 +0200
> @@ -92,13 +92,13 @@ void ConfigureRunCommand(void)
>   #pragma pack(1)
>   struct
>   {
> -  char   ComAux[maxCommand]  __attribute__((packed));
> -  uint32 Options             __attribute__((packed));
> -  uint32 OpsScroll           __attribute__((packed));
> -  uint32 OpsScrHz            __attribute__((packed));
> -  char   Lines[4]            __attribute__((packed));
> -  TListBoxRec tl             __attribute__((packed));
> - } box;
> +  char   ComAux[maxCommand];
> +  uint32 Options;
> +  uint32 OpsScroll;
> +  uint32 OpsScrHz;
> +  char   Lines[4];
> +  TListBoxRec tl;
> + } __attribute__((packed)) box;
>   #pragma pack()
>   strcpy(box.ComAux,Command);
>   box.Options=Options & (~edsmScrollMask);

It works with this patch. Is it the right way?
-- 
Witek