Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 03 Aug 2001 00:38:06 +0100
From:      Brian Somers <brian@Awfulhak.org>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        Julian Elischer <julian@elischer.org>, Brian Somers <brian@Awfulhak.org>, Thomas Pornin <Thomas.Pornin@ens.fr>, freebsd-net@FreeBSD.ORG, freebsd-alpha@FreeBSD.ORG, Brian Somers <brian@freebsd-services.com>, FreeBSD-gnats-submit@FreeBSD.ORG, brian@freebsd-services.com
Subject:   Re: kern/27767: (was: PPPoE + Alpha + 32/64 bits) 
Message-ID:  <200108022338.f72Nc6u09427@hak.lan.Awfulhak.org>
In-Reply-To: Message from Bruce Evans <bde@zeta.org.au>  of "Fri, 03 Aug 2001 06:48:54 %2B1000." <20010803054935.M2890-100000@besplex.bde.org> 

next in thread | previous in thread | raw e-mail | index | archive | help
> `__attribute ((packed))' is a syntax error in Standard C.
> 
> Dependencies on compiler features are supposed to be centralized in
> places like <sys/cdefs.h>.  Many invocations of __attribute__(()) are
> placed there.  This is not possible for the `packed' attribute because
> this attribute has non-cosmetic effects.  Structs must be carefully
> laid out to give some chance of them not having any internal padding
> for any compiler.  This sometimes involves using struct members with
> an unnatural type.  <fs/msdosfs/bpb.h> has many examples.
> 
> Here I don't see how packing the struct helps on alphas.  Doesn't it
> cause traps by misaligning uniqtag.data.pointer?  I think you need
> something like:

It doesn't cause a trap as the compiler is (now) smart enough to 
handle it properly.  Note where we have the alignment problem here:

beast:~ $ cat x.c
#include <stdio.h>

struct s {
  short x, y;
};

union u {
  char c[sizeof(void*)];
  void *v;
};

struct x {
  struct s s;
  union u u;
} __attribute ((packed));

int
main()
{
  struct x x;
  char c[] = "hello";
  int i;

  x.u.v = c;	/* This is ok despite it not being aligned */

  i = (char *)&x.u.v - (char *)&x;
  printf("Got \"%s\", offset %d\n", (char *)x.u.v, i);

  *(void **)((char *)&x + 4) = c;	/* This is not ok */
  printf("Without knowledge of packed... \"%s\"\n", (char *)x.u.v);

  return 0;
}
beast:~ $ cc -o x x.c
beast:~ $ ./x
Got "hello", offset 4
pid 30842 (x): unaligned access: va=0x11ffba64 pc=0x120000894 ra=0x120000884 op=stq
Without knowledge of packed... "hello"


>   	struct {
> 		struct	pppoe_tag hdr;
>  		char	data[sizeof(void *)];
> 	} uniqtag;
> 	void *pointer;
> 	...
> 	pointer = sp;
> 	memcpy(uniqtag.data, &pointer, sizeof(uniqtag.data));

That's probably better.  I went with the __attribute((packed)) stuff 
because it was already done that way in the rest of the code.

In this specific case however, it may be best to just send the variable 
plus the alignment padding as the tag:

        struct {
                struct pppoe_tag hdr;
                union   uniq    data;
        } uniqtag;

        ......
        uniqtag.hdr.tag_type = PTT_HOST_UNIQ;
        uniqtag.hdr.tag_len = htons((u_int16_t)(sizeof(uniqtag) - sizeof(uniqtag.hdr));
        uniqtag.data.pointer = sp;


But I'm not convinced it gains anything given the other uses of 
__attribute((packed)) in this code.

> Bruce

-- 
Brian <brian@freebsd-services.com>                <brian@Awfulhak.org>
      http://www.freebsd-services.com/        <brian@[uk.]FreeBSD.org>
Don't _EVER_ lose your sense of humour !      <brian@[uk.]OpenBSD.org>



To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200108022338.f72Nc6u09427>