|
Home > Archive > Unix Programming > November 2005 > Structure padding
You are viewing an archived Text-only version of the thread.
To view this thread in it's original format and/or if you want to reply to
this thread please [click here]
|
|
| Russell Shaw 2005-11-14, 5:56 pm |
| Hi,
In a memory manager i wrote, i get corruption somewhere from
newly malloced (gnu malloc) memory overwriting existing heap variables.
I have:
----------------------------------------------------------------------------
#include<stddef.h>
#define DATA_PTR_FROM_BLOB(blob) ((void*)(blob) + offsetof(Blob, data))
#define BLOB_FROM_DATA_PTR(ptr) ((void*)(ptr) - offsetof(Blob, data))
typedef struct Blob {
int magic;
size_t payloadsize;
unsigned char data[0];
struct Blob *next, *prev; // overwritten by payload starting at data[0]
} Blob;
----------------------------------------------------------------------------
Macro offsetof() comes from stddef.h (gcc 4.0.3 on debian linux).
I've been doing:
struct Blob ablob;
...
void *dataptr = DATA_PTR_FROM_BLOB(ablob);
...
Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
...
Will this have alignment problems with gaps in the Blob struct?
| |
| Paul Pluzhnikov 2005-11-14, 5:56 pm |
| Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
If the corruption is reproducible, the easiest way to catch it is
with data breakpoint. On P3 and above, gdb implements data
breakpoints via (hardware) debug registers, and they are very fast.
> #define DATA_PTR_FROM_BLOB(blob) ((void*)(blob) + offsetof(Blob, data))
> #define BLOB_FROM_DATA_PTR(ptr) ((void*)(ptr) - offsetof(Blob, data))
Pointer arithmetic on 'void *' is non-portable.
Quick, which of the two expressions below is true?
((void *)p + 1) == ((char *)p + 1)
((void *)p + 1) == ((int *)p + 1)
> Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
>
> Will this have alignment problems with gaps in the Blob struct?
No. The offsetof() takes care of accounting for them.
Cheers,
--
In order to understand recursion you must first understand recursion.
Remove /-nsp/ for email.
| |
| Ben Pfaff 2005-11-14, 5:56 pm |
| Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
>
> typedef struct Blob {
> int magic;
> size_t payloadsize;
> unsigned char data[0];
> struct Blob *next, *prev; // overwritten by payload starting at data[0]
> } Blob;
You declared data[] as having 0 elements. Thus, it overlaps with
next and prev. (Is this really a surprise?)
ANSI C doesn't allow 0-element arrays. You must be using some
compiler extension.
I recommend moving data[] to the end of the struct.
--
Ben Pfaff
email: blp@cs.stanford.edu
web: http://benpfaff.org
| |
| Keith Thompson 2005-11-14, 5:56 pm |
| Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
>
> I have:
>
> ----------------------------------------------------------------------------
> #include<stddef.h>
>
> #define DATA_PTR_FROM_BLOB(blob) ((void*)(blob) + offsetof(Blob, data))
>
> #define BLOB_FROM_DATA_PTR(ptr) ((void*)(ptr) - offsetof(Blob, data))
>
> typedef struct Blob {
> int magic;
> size_t payloadsize;
> unsigned char data[0];
> struct Blob *next, *prev; // overwritten by payload starting at data[0]
> } Blob;
> ----------------------------------------------------------------------------
>
> Macro offsetof() comes from stddef.h (gcc 4.0.3 on debian linux).
>
> I've been doing:
>
> struct Blob ablob;
> ...
> void *dataptr = DATA_PTR_FROM_BLOB(ablob);
> ...
> Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
> ...
>
> Will this have alignment problems with gaps in the Blob struct?
The use of a zero-element array is not allowed in standard C. There's
a more nearly conforming version declares a 1-element array, but
allocates more memory so elements past the end of the array can be
accessed; this is known as the "struct hack". See
<http://www.eskimo.com/~scs/C-faq/q2.6.html> for more information.
But the array must be the last declared member of the struct;
otherwise it will write over any following members.
C99 adds, and I'm fairly sure gcc supports, a conforming version of
the struct hack known as the "flexible array member". The declaration
uses empty brackets rather than the fictitious [0] or [1]:
unsigned char data[];
Search the gcc documentation, or a recent C reference, for "flexible
array member".
--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
| |
| Steve O'Hara-Smith 2005-11-15, 2:51 am |
| On Tue, 15 Nov 2005 03:31:52 +1100
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
> typedef struct Blob {
> int magic;
> size_t payloadsize;
> unsigned char data[0];
> struct Blob *next, *prev; // overwritten by payload starting at data[0]
Well yes and if the payload size is greater than the size of the
two pointers something else is going to get overwritten and you have no
idea what that something else will be.
--
C:>WIN | Directable Mirror Arrays
The computer obeys and wins. | A better way to focus the sun
You lose and Bill collects. | licences available see
| http://www.sohara.org/
| |
| slebetman@yahoo.com 2005-11-15, 2:51 am |
| Russell Shaw wrote:
> Hi,
>
> In a memory manager i wrote, i get corruption somewhere from
> newly malloced (gnu malloc) memory overwriting existing heap variables.
>
> I have:
>
> ----------------------------------------------------------------------------
> #include<stddef.h>
>
> #define DATA_PTR_FROM_BLOB(blob) ((void*)(blob) + offsetof(Blob, data))
>
> #define BLOB_FROM_DATA_PTR(ptr) ((void*)(ptr) - offsetof(Blob, data))
>
> typedef struct Blob {
> int magic;
> size_t payloadsize;
> unsigned char data[0];
> struct Blob *next, *prev; // overwritten by payload starting at data[0]
> } Blob;
> ----------------------------------------------------------------------------
>
> Macro offsetof() comes from stddef.h (gcc 4.0.3 on debian linux).
>
> I've been doing:
>
> struct Blob ablob;
> ...
> void *dataptr = DATA_PTR_FROM_BLOB(ablob);
> ...
> Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
> ...
>
> Will this have alignment problems with gaps in the Blob struct?
Others have already explained this, but just to make it clear I think
what you want is this:
typedef struct Blob {
int magic;
size_t payloadsize;
struct Blob *next, *prev;
unsigned char data[0];
} Blob;
Happened to me once when I had to add a member to my struct. Always
declare stuff BEFORE the data section. Otherwise the member variable
and data will corrupt each other.
| |
| Paul Pluzhnikov 2005-11-15, 2:51 am |
| "slebetman@yahoo.com" <slebetman@gmail.com> writes:
> Others have already explained this, but just to make it clear I think
> what you want is this:
He probably doesn't. When writing a memory manager, it is a *very*
common technique to have a "Blob" either allocated, or on the
free-list.
Thus it makes sense to share the storage for forward/back links and
the user data, because either one or the other is used at any given
moment, but never both at the same time.
I think the OP did this *intentionally* (and provided a comment
stating this intent), and everybody who pointed out that data[]
would overwrite the links missed the point. But only the OP can
say for sure.
Cheers,
--
In order to understand recursion you must first understand recursion.
Remove /-nsp/ for email.
| |
| Russell Shaw 2005-11-15, 7:54 am |
| Paul Pluzhnikov wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
>
>
> If the corruption is reproducible, the easiest way to catch it is
> with data breakpoint. On P3 and above, gdb implements data
> breakpoints via (hardware) debug registers, and they are very fast.
>
>
> Pointer arithmetic on 'void *' is non-portable.
> Quick, which of the two expressions below is true?
>
> ((void *)p + 1) == ((char *)p + 1)
> ((void *)p + 1) == ((int *)p + 1)
My only assumption is that a pointer to a void assumes the basic increment
of the pointer is one byte, and that char is also one byte. To make it
portable, you let autoconf set some #defines. Therefore the first is true
because sizeof(int) is usually 4-bytes or more on PCs.
>
> No. The offsetof() takes care of accounting for them.
>
> Cheers,
| |
| Russell Shaw 2005-11-15, 7:54 am |
| Ben Pfaff wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
>
>
> You declared data[] as having 0 elements. Thus, it overlaps with
> next and prev. (Is this really a surprise?)
>
> ANSI C doesn't allow 0-element arrays. You must be using some
> compiler extension.
I'm using "data" as an empty marker for the first free memory location following
that point. Anything written starting at "data" will overwrite next/prev,
and extend outside the struct. It's a bit like using alloca(0), but i'm doing
it on the heap.
> I recommend moving data[] to the end of the struct.
| |
| Russell Shaw 2005-11-15, 7:54 am |
| Keith Thompson wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
>
>
> The use of a zero-element array is not allowed in standard C. There's
> a more nearly conforming version declares a 1-element array, but
> allocates more memory so elements past the end of the array can be
> accessed; this is known as the "struct hack". See
> <http://www.eskimo.com/~scs/C-faq/q2.6.html> for more information.
I'm using a gcc extension for zero length arrays.
> But the array must be the last declared member of the struct;
> otherwise it will write over any following members.
When Blob is taken out of storage for use, user data starts at "data",
overwriting next/prev pointers. Those pointers are only used when the
blob is back in the pool, so 8 bytes are gained for free during use.
> C99 adds, and I'm fairly sure gcc supports, a conforming version of
> the struct hack known as the "flexible array member". The declaration
> uses empty brackets rather than the fictitious [0] or [1]:
> unsigned char data[];
> Search the gcc documentation, or a recent C reference, for "flexible
> array member".
5.11 in gcc-4.0 info explains it. Unfortunately, it says:
* Flexible array members may only appear as the last member of a
`struct' that is otherwise non-empty.
| |
| Russell Shaw 2005-11-15, 7:54 am |
| Ben Pfaff wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
>
>
> You declared data[] as having 0 elements. Thus, it overlaps with
> next and prev. (Is this really a surprise?)
>
> ANSI C doesn't allow 0-element arrays. You must be using some
> compiler extension.
I'm using gcc-4. I don't see why they should disallow empty array decs.,
because it's good for marking a memory location used for variable size
storage.
> I recommend moving data[] to the end of the struct.
| |
| Russell Shaw 2005-11-15, 7:54 am |
| Steve O'Hara-Smith wrote:
> On Tue, 15 Nov 2005 03:31:52 +1100
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
>
>
> Well yes and if the payload size is greater than the size of the
> two pointers something else is going to get overwritten and you have no
> idea what that something else will be.
Blob is actually the head of a variable size memory chunk, with an arbitrary
size payload starting at "data".
| |
| Russell Shaw 2005-11-15, 7:54 am |
| Paul Pluzhnikov wrote:
> "slebetman@yahoo.com" <slebetman@gmail.com> writes:
>
>
> He probably doesn't. When writing a memory manager, it is a *very*
> common technique to have a "Blob" either allocated, or on the
> free-list.
>
> Thus it makes sense to share the storage for forward/back links and
> the user data, because either one or the other is used at any given
> moment, but never both at the same time.
>
> I think the OP did this *intentionally* (and provided a comment
> stating this intent), and everybody who pointed out that data[]
> would overwrite the links missed the point. But only the OP can
> say for sure.
>
> Cheers,
This is what i'm doing. I've since determined that the bug wasn't
even in the memory manager stuff. I thought it was very likely to
be here, because it has only been lightly tested. I was extending
a memory segment from my main app, but realloc() can move all the
data to some place else, thereby invalidating all pointers that
were pointing there;)
| |
| Chris Dollin 2005-11-15, 7:54 am |
| Russell Shaw wrote:
> When Blob is taken out of storage for use, user data starts at "data",
> overwriting next/prev pointers. Those pointers are only used when the
> blob is back in the pool, so 8 bytes are gained for free during use.
Was there something wrong with using a union to express this?
--
Chris "another virtual machine" Dollin
time is just space under pressure. squeeze!
| |
| Jordan Abel 2005-11-15, 7:54 am |
| On 2005-11-15, Paul Pluzhnikov <ppluzhnikov-nsp@charter.net> wrote:
> "slebetman@yahoo.com" <slebetman@gmail.com> writes:
>
>
> He probably doesn't. When writing a memory manager, it is a *very*
> common technique to have a "Blob" either allocated, or on the
> free-list.
>
> Thus it makes sense to share the storage for forward/back links and
> the user data, because either one or the other is used at any given
> moment, but never both at the same time.
>
> I think the OP did this *intentionally* (and provided a comment
> stating this intent), and everybody who pointed out that data[]
> would overwrite the links missed the point. But only the OP can
> say for sure.
I'd suggest a union to make it explicit then.
| |
| Steve O'Hara-Smith 2005-11-15, 5:59 pm |
| On Tue, 15 Nov 2005 19:36:38 +1100
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
> Steve O'Hara-Smith wrote:
>
> Blob is actually the head of a variable size memory chunk, with an arbitrary
> size payload starting at "data".
It didn't look like it in the code fragment you posted viz:
struct Blob ablob;
...
void *dataptr = DATA_PTR_FROM_BLOB(ablob);
...
Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
Now if you had something like:
struct Blob *ablob = malloc (ENOUGH_SPACE)
....
you might get somewhere - but I can't see why you want to overwrite
the next and prev pointers, as others have suggested the variable length
array should be at the end (and for portability declared as data[] or
data[1]).
--
C:>WIN | Directable Mirror Arrays
The computer obeys and wins. | A better way to focus the sun
You lose and Bill collects. | licences available see
| http://www.sohara.org/
| |
| Giorgos Keramidas 2005-11-15, 5:59 pm |
| Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> Paul Pluzhnikov wrote:
>
> My only assumption is that a pointer to a void assumes the basic increment
> of the pointer is one byte, and that char is also one byte. To make it
> portable, you let autoconf set some #defines. Therefore the first is true
> because sizeof(int) is usually 4-bytes or more on PCs.
You can't depend on that though. It's 8 bytes here on a "modern"
system, running FreeBSD/amd64
| |
| Keith Thompson 2005-11-15, 5:59 pm |
| Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
>
> You can't depend on that though. It's 8 bytes here on a "modern"
> system, running FreeBSD/amd64
I think he knows that, which is why he wrote "4-bytes or more".
Strictly speaking, the only relevant requirements imposed by the C
language are sizeof(char)==1, sizeof(int)>=1 (and if sizeof(int)==1,
CHAR_BIT>=16) -- but I don't think gcc supports any such targets.
Assuming the gcc-specific semantics for artithmetic on void*, the
first expression above is true, and the second is false (unless
sizeof(int)==1).
Of course, if you use arithmetic on void*, you've left the realm of
portable standard C, and your code probably won't compile with
compilers other than gcc. That's fine if you're willing to pay that
price.
--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
| |
| Keith Thompson 2005-11-15, 5:59 pm |
| Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
> Ben Pfaff wrote:
>
> I'm using gcc-4. I don't see why they should disallow empty array decs.,
> because it's good for marking a memory location used for variable size
> storage.
They can be useful, but they're non-standard, and any code that uses
them isn't portable. (gcc warns about this with "-pedantic".)
If the C standard supported zero-length arrays, I would have no
objection at all. As it is, it's up to you to decide whether the
functionality is worth the loss of portability.
--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
| |
| Roger Willcocks 2005-11-15, 5:59 pm |
| "Russell Shaw" <rjshawN_o@s_pam.netspace.net.au> wrote in message
news:ooqm43-2o4.ln1@main.anatron.com.au...
> Hi,
> I Have:
> #include<stddef.h>
> #define DATA_PTR_FROM_BLOB(blob) ((void*)(blob) + offsetof(Blob, data))
Me puzzled:
int main(int argc, char* argv[])
{
return (int)((void *)argv[0] + 10);
}
MSVC says "error C2036: 'void *' : unknown size' while gcc accepts this
happily. Is it valid or not?
--
Roger
| |
| Erik Max Francis 2005-11-15, 8:50 pm |
| Roger Willcocks wrote:
> Me puzzled:
>
> int main(int argc, char* argv[])
> {
> return (int)((void *)argv[0] + 10);
> }
>
> MSVC says "error C2036: 'void *' : unknown size' while gcc accepts this
> happily. Is it valid or not?
Pointer arithmetic doesn't make any sense on a void *, so it's invalid.
--
Erik Max Francis && max@alcyone.com && http://www.alcyone.com/max/
San Jose, CA, USA && 37 20 N 121 53 W && AIM erikmaxfrancis
You could have another fate / You could be in another place
-- Anggun
| |
| Keith Thompson 2005-11-15, 8:50 pm |
| "Roger Willcocks" <rkww@rops.org> writes:
> "Russell Shaw" <rjshawN_o@s_pam.netspace.net.au> wrote in message
> news:ooqm43-2o4.ln1@main.anatron.com.au...
>
> Me puzzled:
>
> int main(int argc, char* argv[])
> {
> return (int)((void *)argv[0] + 10);
> }
>
> MSVC says "error C2036: 'void *' : unknown size' while gcc accepts this
> happily. Is it valid or not?
Standard C does not support pointer arithmetic. gcc does support it
as a language extension. (In my opinion it's a bad idea, but it is a
feature of the nonstandard "GNU C" language whether I like it or not.)
Try "gcc -pedantic".
--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
| |
|
| Keith Thompson wrote:
> Standard C does not support pointer arithmetic.
There's not enough words in that sentence.
--
pete
| |
| Russell Shaw 2005-11-15, 8:50 pm |
| Steve O'Hara-Smith wrote:
> On Tue, 15 Nov 2005 19:36:38 +1100
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
>
>
> It didn't look like it in the code fragment you posted viz:
>
> struct Blob ablob;
> ...
> void *dataptr = DATA_PTR_FROM_BLOB(ablob);
> ...
> Blob *blobheader = BLOB_FROM_DATA_PTR(dataptr);
>
> Now if you had something like:
>
> struct Blob *ablob = malloc (ENOUGH_SPACE)
> ...
>
> you might get somewhere - but I can't see why you want to overwrite
> the next and prev pointers, as others have suggested the variable length
> array should be at the end (and for portability declared as data[] or
> data[1]).
static Blob*
blob_malloc(size_t size)
{
Blob *blob = malloc(sizeof(Blob) + size);
blob->payloadsize = size; /* data-size following blob */
return blob;
}
blob_malloc(size) would be 8 bytes larger if the space occupied by
the next/prev pointers weren't over written (32-bit pointers).
After doing a blob_malloc(), next/prev pointers are no longer needed.
| |
| Russell Shaw 2005-11-15, 8:50 pm |
| Giorgos Keramidas wrote:
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> writes:
>
>
> You can't depend on that though. It's 8 bytes here on a "modern"
> system, running FreeBSD/amd64
That's what Autoconf is for. You can adjust your code to adapt
to the install environment.
| |
| Russell Shaw 2005-11-15, 8:50 pm |
| Russell Shaw wrote:
> Steve O'Hara-Smith wrote:
>
>
>
> static Blob*
> blob_malloc(size_t size)
> {
> Blob *blob = malloc(sizeof(Blob) + size);
> blob->payloadsize = size; /* data-size following blob */
> return blob;
> }
>
> blob_malloc(size) would be 8 bytes larger if the space occupied by
> the next/prev pointers weren't over written (32-bit pointers).
> After doing a blob_malloc(), next/prev pointers are no longer needed.
Correction:
blob_malloc(size) would be 8 bytes larger if the space occupied by
the next/prev pointers weren't over written (32-bit pointers).
After doing a blob_malloc(), the next/prev pointers are used to put
the blob into a linked list. After delivery to the user app, the
next/prev pointers are no longer needed.
| |
| Giorgos Keramidas 2005-11-16, 2:49 am |
| Erik Max Francis <max@alcyone.com> writes:
> Roger Willcocks wrote:
>
> Pointer arithmetic doesn't make any sense on a void *, so it's invalid.
GCC *does* warn about it, too:
flame:/tmp$ gcc -ansi -pedantic foo.c
foo.c: In function `main':
foo.c:3: warning: pointer of type `void *' used in arithmetic
foo.c:3: warning: cast from pointer to integer of different size
flame:/tmp$
| |
| Keith Thompson 2005-11-16, 7:54 am |
| pete <pfiland@mindspring.com> writes:
> Keith Thompson wrote:
>
>
> There's not enough words in that sentence.
D'oh!
Standard C does not support pointer arithmetic on type void*.
--
Keith Thompson (The_Other_Keith) kst-u@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
| |
| Casper H.S. Dik 2005-11-16, 7:54 am |
| "Roger Willcocks" <rkww@rops.org> writes:
>"Russell Shaw" <rjshawN_o@s_pam.netspace.net.au> wrote in message
>news:ooqm43-2o4.ln1@main.anatron.com.au...
[vbcol=seagreen]
>Me puzzled:
>int main(int argc, char* argv[])
>{
> return (int)((void *)argv[0] + 10);
>}
>MSVC says "error C2036: 'void *' : unknown size' while gcc accepts this
>happily. Is it valid or not?
It is invalid; gcc is broken.
Casper
--
Expressed in this posting are my opinions. They are in no way related
to opinions held by my employer, Sun Microsystems.
Statements on Sun products included here are not gospel and may
be fiction rather than truth.
| |
| Casper H.S. Dik 2005-11-16, 7:54 am |
| Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>Erik Max Francis <max@alcyone.com> writes:
[vbcol=seagreen]
>GCC *does* warn about it, too:
>flame:/tmp$ gcc -ansi -pedantic foo.c
>foo.c: In function `main':
>foo.c:3: warning: pointer of type `void *' used in arithmetic
>foo.c:3: warning: cast from pointer to integer of different size
It shouldn't accept it, though. This type of "extensions" makes for
programmers writing non-portable code.
"embrace and extend".
Casper
--
Expressed in this posting are my opinions. They are in no way related
to opinions held by my employer, Sun Microsystems.
Statements on Sun products included here are not gospel and may
be fiction rather than truth.
| |
| Steve O'Hara-Smith 2005-11-16, 7:54 am |
| On Wed, 16 Nov 2005 13:31:33 +1100
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
> Russell Shaw wrote:
[vbcol=seagreen]
Ah I grok it now - but shouldn't the malloc be for
sizeof(Blob) + size - 8 or the payloadsize be size + 8 ? Or better
rather than 8, 2*sizeof(Blob *).
I would also be inclined to use a union to express this rather
than a zero length array.
--
C:>WIN | Directable Mirror Arrays
The computer obeys and wins. | A better way to focus the sun
You lose and Bill collects. | licences available see
| http://www.sohara.org/
| |
| Russell Shaw 2005-11-16, 7:54 am |
| Steve O'Hara-Smith wrote:
> On Wed, 16 Nov 2005 13:31:33 +1100
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
>
>
>
> Ah I grok it now - but shouldn't the malloc be for
> sizeof(Blob) + size - 8 or the payloadsize be size + 8 ? Or better
> rather than 8, 2*sizeof(Blob *).
What really happens that is not shown, is that malloc(sizeof(Blob))
is done when 8 bytes or less are wanted. For more bytes,
malloc(sizeof(Blob) + size - 2*sizoef(void*)) is done.
> I would also be inclined to use a union to express this rather
> than a zero length array.
| |
| Steve O'Hara-Smith 2005-11-16, 6:03 pm |
| On Wed, 16 Nov 2005 22:49:24 +1100
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
> Steve O'Hara-Smith wrote:
>
> What really happens that is not shown, is that malloc(sizeof(Blob))
> is done when 8 bytes or less are wanted. For more bytes,
> malloc(sizeof(Blob) + size - 2*sizoef(void*)) is done.
Y'know it would have caused less confusion if you'd posted
snippets of the real code originally. Granted we probably wouldn't
have seen the realloc problem but at least we wouldn't have chased
so many phantoms.
--
C:>WIN | Directable Mirror Arrays
The computer obeys and wins. | A better way to focus the sun
You lose and Bill collects. | licences available see
| http://www.sohara.org/
| |
| Russell Shaw 2005-11-16, 6:03 pm |
| Steve O'Hara-Smith wrote:
> On Wed, 16 Nov 2005 22:49:24 +1100
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
>
>
>
> Y'know it would have caused less confusion if you'd posted
> snippets of the real code originally. Granted we probably wouldn't
> have seen the realloc problem but at least we wouldn't have chased
> so many phantoms.
It would have been interesting, but distracting noise, because it
was very unlikely the bug was in there. It also needs more testing,
and optimizing. It may be released one day anyway.
| |
| Wayne C. Morris 2005-11-16, 6:03 pm |
| In article <5vir43-v45.ln1@main.anatron.com.au>,
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
> Steve O'Hara-Smith wrote:
>
> What really happens that is not shown, is that malloc(sizeof(Blob))
> is done when 8 bytes or less are wanted. For more bytes,
> malloc(sizeof(Blob) + size - 2*sizoef(void*)) is done.
I'd use (offsetof(Blob,data)+size), and test to make sure it isn't less
than sizeof(Blob). It's simpler, and doesn't rely on knowing the number
and types of the temporary fields following data[], so it'd be easier to
add/remove temporary fields if you ever want to.
Elsewhere you noted that the GCC documentation says flexible array members
must appear as the last member of a struct. I assume after you found that,
you changed your struct to use a 1-element array?
Getting back to your original post, I'd avoid using non-portable void
pointer arithmetic by changing the macros to:
#define DATA_PTR_FROM_BLOB(blob) ((void*) ((blob)->data))
#define BLOB_FROM_DATA_PTR(ptr) ((void*) ((char*)(ptr) \
- offsetof(Blob, data)))
Is there any chance the application might use data[] to store anything
other than an array of char? If so, you should change the structure
definition to ensure data[] is properly aligned for any data type.
| |
| Giorgos Keramidas 2005-11-16, 6:03 pm |
| Casper H.S. Dik <Casper.Dik@Sun.COM> writes:
> Giorgos Keramidas <keramida@ceid.upatras.gr> writes:
>
>
>
> It shouldn't accept it, though. This type of "extensions" makes for
> programmers writing non-portable code.
>
> "embrace and extend".
In -pedantic mode this is a warning. Using -Werror makes all warnings
fatal too, so it *is* possible to write portable code with GCC that
doesn't use the 'extensions'. It's just the defaults that are broken.
Despite being a very good set of tools, the Sun Studio 10 compiler may
accept code that is not 100% portable when used without the -Xc
option. I don't that makes the compiler broken in any way though.
Cautious & careful programmers can still use both compilers to write
perfectly portable code.
| |
| Brian Raiter 2005-11-16, 6:03 pm |
| > It shouldn't accept it, though. This type of "extensions" makes for
> programmers writing non-portable code.
If your goal is to write portable code, then use the -ansi and
-pedantic cmdline arguments. The compiler can't intuit how you want to
use it.
In the end, you can't rely on the compiler to help you write portable
code. That has to be your job. It can point out some of the more
egregious problems, but the way the C standard is defined, it is
simply impossible for one C compiler to prevent you from using all
potentially non-portable constructs.
b
| |
| Russell Shaw 2005-11-16, 8:48 pm |
| Wayne C. Morris wrote:
> In article <5vir43-v45.ln1@main.anatron.com.au>,
> Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
>
>
> I'd use (offsetof(Blob,data)+size), and test to make sure it isn't less
> than sizeof(Blob). It's simpler, and doesn't rely on knowing the number
> and types of the temporary fields following data[], so it'd be easier to
> add/remove temporary fields if you ever want to.
>
> Elsewhere you noted that the GCC documentation says flexible array members
> must appear as the last member of a struct. I assume after you found that,
> you changed your struct to use a 1-element array?
No;)
I find the lack of zero-length arrays mathematically displeasing;)
When memory allocations are handled algorithmically, it is a good
idea to have blob_malloc(0) return a valid memory object. In the algorithm,
some blobs may be shortened to 0, while other zero-length ones are
lengthend. Requiring data[1] means that blob_malloc(0) will always be
larger than neccessary.
If portability to another compiler is required, i'll think of the specific
problem then. I prefer to think of the lack of zero-length arrays more as
a bug that only exists for backward compatability. It will all work with
data[1], but it's just so "incomplete";)
> Getting back to your original post, I'd avoid using non-portable void
> pointer arithmetic by changing the macros to:
>
> #define DATA_PTR_FROM_BLOB(blob) ((void*) ((blob)->data))
>
> #define BLOB_FROM_DATA_PTR(ptr) ((void*) ((char*)(ptr) \
> - offsetof(Blob, data)))
>
> Is there any chance the application might use data[] to store anything
> other than an array of char? If so, you should change the structure
> definition to ensure data[] is properly aligned for any data type.
It will store all kinds of junk. GCC has alignment attributes i could use.
| |
| Wayne C. Morris 2005-11-17, 6:13 pm |
| In article <ns4t43-dl1.ln1@main.anatron.com.au>,
Russell Shaw <rjshawN_o@s_pam.netspace.net.au> wrote:
>
> No;)
[snip]
> Requiring data[1] means that blob_malloc(0) will always be
> larger than neccessary.
Not if you use a union to ensure 'data' and 'next' start at the same offset
within the structure, like so:
union {
unsigned char data[1];
struct {
// overwritten by payload starting at data[0]
struct Blob *next, *prev;
} p;
} u;
Or you could make 'data' a nested structure containing 'next' and 'prev':
struct {
// overwritten by payload starting at 'data'
struct Blob *next, *prev;
} data;
Or you could eliminate 'data' entirely, and use 'next' in its place:
#define DATA_PTR_FROM_BLOB(blob) ((void*) &(blob)->next)
#define BLOB_FROM_DATA_PTR(ptr) ((void*) ((char*)(ptr) \
- offsetof(Blob, next)))
Or you could move the pointers into a separate 'BlobLinks' structure, and
access it via ((BlobLinks*) &blob->data[0]). This would allow you to have
your zero-length array at the end of the structure, and make it easy to
test whether the requested allocation size is smaller than 'BlobLinks'.
|
|
|
|
|