|
|
|
#1 |
|
Messages: n/a
Hébergeur: |
Ok, I'm having 'fun' with pointers to structures.
I've got some code that looks something like this: ==================== typedef struct { unsigned long *clno, *lastHistoryRecord; } aRecord; .... .... void main() { .... .... theRecord aRecord; theRecord.clno = (unsigned long*) malloc(sizeof(long)); theRecord.lastHistoryRecord = (unsigned long*) malloc(sizeof(long)); getArecord( infile, &theRecord, recordNumber ); printf( "%ld %ld\n", theRecord.clno, theRecord.lastHistoryRecord ); .... .... } int getArecord( FILE *infile, aRecord *thisRecord, long recordNumber ) { .... .... fread( &thisRecord->lastHistoryRecord, 1L, sizeof(long), infile ); fread( &thisRecord->clno, 1L, sizeof( long ) * 1, infile ); .... .... return result; } ==================== (large chunks of code snipped for brevity) Now, it _seems_ to do as I expect. Let's say that I run this and get: 1000 1234 as a result. If I change the printf line to say: printf( "%ld %ld\n", theRecord.clno-1, theRecord.lastHistoryRecord ); then I get 996 1234. I would have expected 999. So I'm obviously doing something very, very stupid somewhere[0]. Could someone enlighten me? FWIW I've also tried --theRecord.clno and (theRecord.clno)-1 but they both do the same. Many thanks. Sorry if this is a really, really obvious one..! Jim [0] apart from not checking the mallocs for errors -- http://www.ursaMinorBeta.co.uk BANTEER (n. archaic) A lusty and raucous old ballad sung after a particularly spectacular araglin (q.v.) has been pulled off. |
|
|
|
#2 |
|
Messages: n/a
Hébergeur: |
Jim wrote:
> typedef struct > { > unsigned long *clno, *lastHistoryRecord; > } aRecord; So structures of this type contain pointers to unsigned longs - why not just longs? > theRecord aRecord; > > theRecord.clno = (unsigned long*) malloc(sizeof(long)); > theRecord.lastHistoryRecord = (unsigned long*) malloc(sizeof(long)); That makes some sense, though I'd rather you at least said "malloc(sizeof(unsigned long))", and dropped the cast (not ful). The c.l.c preferred form would be :- theRecord.clno = malloc(sizeof(*theRecord.clno)); etc... > fread( &thisRecord->lastHistoryRecord, 1L, sizeof(long), infile ); > fread( &thisRecord->clno, 1L, sizeof( long ) * 1, infile ); Here you pass fread() the _addresses_ of the pointers to long. So the data read from the file is put there. Either you need to change this code to remove the & characters, or change the structure to contain un > printf( "%ld %ld\n", theRecord.clno, theRecord.lastHistoryRecord ); This passes printf() two pointers and tells it it has two longs - this is clearly wrong. > 1000 1234 But you get away with it. > as a result. If I change the printf line to say: > > printf( "%ld %ld\n", theRecord.clno-1, theRecord.lastHistoryRecord ); > > then I get > > 996 1234. theRecord.clno is a pointer to unsigned long. Subtracting 1 from it moves it "back" (for some value of back) by sizeof(unsigned long). Again you pass printf() two pointers but tell it that they are longs, again you "get away" with it... Your code would work acceptably simply by amending the structure to contain unsigned longs, rather than pointers to them, and removing the mallocs... |
|
|
|
#3 |
|
Messages: n/a
Hébergeur: |
On 2007-10-17, Mark Bluemel <mark_bluemel@pobox.com> wrote:
> > theRecord.clno is a pointer to unsigned long. Subtracting 1 from it > moves it "back" (for some value of back) by sizeof(unsigned long). Again > you pass printf() two pointers but tell it that they are longs, again > you "get away" with it... Barely, it would seem. > Your code would work acceptably simply by amending the structure to > contain unsigned longs, rather than pointers to them, and removing the > mallocs... Done and it now works as expected. Many thanks, and I'll try to get this into my head in future. Jim -- http://www.ursaMinorBeta.co.uk BRADWORTHY (n.) One who is skilled in the art of naming loaves. |
|
|
|
#4 |
|
Messages: n/a
Hébergeur: |
On Wed, 17 Oct 2007 13:32:03 +0100, Jim <jim@magrathea.plus.com>
wrote: >Ok, I'm having 'fun' with pointers to structures. > >I've got some code that looks something like this: > >==================== >typedef struct >{ > unsigned long *clno, *lastHistoryRecord; >} aRecord; This defines a **type** known as aRecord. > >... >... > >void main() main returns int. >{ >... >... > theRecord aRecord; This is a syntax error. There is no type known as theRecord. Perhaps you would like to show us your real code. Use cut and paste. Do not attempt to retype it. > > theRecord.clno = (unsigned long*) malloc(sizeof(long)); Don't cast the return from malloc. The only thing it accomplishes is preventing the compiler from issuing a diagnostic which you would really want to see. It is always desirable that the operand of sizeof match the type of the object being pointed to. long and unsigned long may have the same size but it is a better style. To this end, the frequent recommendation here is theRecord.clno = malloc(sizeof *theRecord.clno); which will still be correct if you later need to change the type of clno. > theRecord.lastHistoryRecord = (unsigned long*) malloc(sizeof(long)); > > getArecord( infile, &theRecord, recordNumber ); > > printf( "%ld %ld\n", theRecord.clno, theRecord.lastHistoryRecord ); This is undefined behavior. Neither the second nor third arguments match the type of the respective format specification. Both are pointers where the format requires a long. If you meant to print the pointer values (unlikely), use %p and cast the arguments to void*. If you meant to print the values pointed to, then apply the dereference operator (*) to both arguments. >... >... >} > > >int getArecord( FILE *infile, aRecord *thisRecord, long recordNumber ) You did know the correct type name. >{ >... >... > fread( &thisRecord->lastHistoryRecord, 1L, sizeof(long), infile ); This will work only if the data in infile was produced on a system which stores long values EXACTLY the same as the system you are running this program on. > fread( &thisRecord->clno, 1L, sizeof( long ) * 1, infile ); >... >... >return result; >} > >==================== > >(large chunks of code snipped for brevity) > > >Now, it _seems_ to do as I expect. Let's say that I run this and get: > >1000 1234 By very bad luck, the undefined behavior printed pointer values that very much resemble integers. > >as a result. If I change the printf line to say: > >printf( "%ld %ld\n", theRecord.clno-1, theRecord.lastHistoryRecord ); > >then I get > >996 1234. I would have expected 999. So I'm obviously doing something very, By equally bad luck, sizeof(long) on your system is 4 and the pointer arithmetic resulted in another value that looks like an integer. >very stupid somewhere[0]. Could someone enlighten me? FWIW I've also tried >--theRecord.clno and (theRecord.clno)-1 but they both do the same. > >Many thanks. Sorry if this is a really, really obvious one..! You want *theRecord.clno-1. This dereferences the pointer to obtain the long value and subtracts one from that value, not from the pointer. > >Jim >[0] apart from not checking the mallocs for errors Remove del for email |
|
![]() |
| Outils de la discussion | |
|
|