Help the helpless.. code read

[Mark Richards]

I've been programming for many years, but have only recently taken a
deep "C" dive (bad pun, i know) and need a lot of explanation from an
expert. My questions center around those mysterious pointer beasties.

The following code is excerpted from digitemp, a program that reads
1-wire devices. Digitemp is the work of Brian C. Lane.

As I walk through this code, I have a number of questions. I'm hoping
someone with experience in pointers can provide a few for me (another
pun) Also, I am sorry that this message is long. I tried to
shorten it, but could not get my questions across any other way.

I have indented the code and my questions and comments are preceded by
// so they will be easier to follow. I hope this is less confusing and
respectful of everyone's time.


// The following _roms structure contains a list of 8 byte addresses...

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

// A variable, sensor_list is initialized...

struct _roms *sensor_list

// The roms part of the structure is filled with an "array" of sorts,
// containing one or more 8-byte addresses. These addresses resolve,
// when printed, as hex values.

// For example: "12BAA91C000000A3" is a valid address string.

// The other variable within the _roms structure is "max" which is
// loaded with the total number of addresses stored in "roms".

// As I understand it, *sensor_list as defined above is a pointer to the
// beginning of the entire structure defined as _roms.

// Is this correct?


// Let's say sensor_list is loaded with one 8-byte-wide address:
// "12BAA91C000000A3".

// Here is how it's passed through in the code:

read_all(&sensor_list);

// Above, read_all(&sensor_list) says "call the read_all function and
// pass as a parameter the (starting) address where the variable
// sensor_list is. Is this a correct reading?

int read_all(struct _roms *sensor_list)
{
int x;
for(x = 0; x < (num_cs + sensor_list->max); x++)
{
read_device(sensor_list, x);
}
return 0;
}

// It appears that a de-referenced pointer to sensor_list is being
// defined in the function header, correct? (Since the address of
// sensor_list was passed in the call to read_all(&sensor_list).)

// From this, is it correct to assume that further de-referencing of
// sensor_list will not be needed?

// Here's where the action takes place:

int read_device(struct _roms *sensor_list, int sensor)
{
...
}

// What seems odd to me is that, again, sensor_list is being defined in
// this function header as a dereferenced pointer. Yet, the calling
// function used a variable (sensor_list) that had already been
// de-referenced. That's why it called read_device with just
// "sensor_list", correct?

// So there is a fundamental here that I am not understanding.

// Within read_device, the sensor_list structure is used to pass a
// complete 8-byte address to a function called owSerialNum:

owSerialNum( 0, &sensor_list->roms[sensor*8], FALSE );

// If I call the function like this:

owSerialNum( 0, "12BAA91C000000A3", FALSE );

// it works perfectly. The documentation for owSerialNum says that the
// second parameter is a buffer containing the address.

// I can see that &sensor_list->roms[sensor*8] is saying, "the address
// of roms[beginning at this offset] within a structure we call
// "sensor_list".

// Since the second parameter in
// owSerialNum( 0,"12BAA91C000000A3",FALSE); is passed through as the
// starting address of a buffer, and since &sensor_list->roms[sensor*8]
// is also the address of a buffer, they must be synonymous, correct?

// Well, they appear not to be so.
// The following code works:

printf(" Address: %s","12BAA91C000000A3");

// This prints garbage:

printf(" Address: %s",&sensor_list->roms[sensor*8] );

// In fact, it looks like it's printing a memory address instead of the
// contents of that address. Why?

// If I do this...

printf("Sensor here: %s\n",sensor_list->roms[sensor*8]);

// I get a segmentation fault (crash).
// If I do this:

printf("Sensor here: %s\n",*sensor_list->roms[sensor*8]);

// The compiler complains: invalid type argument of `unary *'


*********

What I eventually want to do is pass into the application an address as
a string parameter, and be able to pass it into the read_device
function so that it is properly passed to owSerialNum(). It seems
simple enough, but I'm hung up on the above issues at the moment. If I
can be guided through the mud, this once, I think I'll have it.


Help!

 

[Allin Cottrell]

I've been programming for many years, but have only recently taken a
deep "C" dive (bad pun, i know) and need a lot of explanation from an
expert. My questions center around those mysterious pointer beasties.

I'll tackle a small portion of your question.

// The following _roms structure contains a list of 8 byte addresses...

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

// A variable, sensor_list is initialized...

struct _roms *sensor_list

Registered. Although this line is not valid C as it stands. It
needs a semi-colon as termination, possibly preceded by an
actual initialization.

// As I understand it, *sensor_list as defined above is a pointer to the
// beginning of the entire structure defined as _roms.

Not quite: sensor_list is declared as having the type, "pointer to
struct _roms". "*sensor_list", that is the thing you get when you
dereference sensor_list, then has type struct _roms.

// Here is how it's passed through in the code:

read_all(&sensor_list);

// Above, read_all(&sensor_list) says "call the read_all function and
// pass as a parameter the (starting) address where the variable
// sensor_list is. Is this a correct reading?

Yes, it's correct.

int read_all(struct _roms *sensor_list)

Stop right there. Unless there's some slippage in your quotation
from the original C source, this is an error. "sensor_list" was
of type "pointer to struct _roms". The thing you get by writing
"&sensor_list" is then of type "pointer to pointer to struct _roms",
or struct _roms **. And this is not compatible with the stated
parameter type for read_all().

Allin Cottrell
Wake Forest University

 

[CBFalconer]

.... snip ...

I have indented the code and my questions and comments are
preceded by // so they will be easier to follow. I hope this is
less confusing and respectful of everyone's time.

// The following _roms structure contains a list of 8 byte
addresses...

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

Let's stop right there, as the rest of the assumptions will be
wrong. roms is a pointer to an unsigned char. It is not an array
of any sort. I read no further.

In addition the identifier '_roms' is reserved for the
implementation. You should not be using it. Such use may cause
undefined behaviour.

 

[Mark Richards]

In addition the identifier '_roms' is reserved for the

> implementation. You should not be using it. Such use may cause
> undefined behaviour.

When you say it's reserved for the implementation, do you mean that it's
possibly a reserved word, or that the underscore should not be used?
If I define:

struct _roms *sensor_list

then the variable sensor_list acts as a pointer to the structure, correct?

 

[Chris Dollin]

When you say it's reserved for the implementation, do you mean that it's
possibly a reserved word, or that the underscore should not be used?

Sufficiently many identifier-like things beginning with _ are reserved
to the implementation that the easiest rule to remember is "don't use
them".

The implementation is free to attach any meaning it pleases to such
words. (I can't remember whether it's obliged to document those
meanings.) In particular it may use them as struct tags for its own
structures such as might be hidden in #include files.

If I define:

struct _roms *sensor_list

then the variable sensor_list acts as a pointer to the structure, correct?

The variable sensor_list, when its value is required, should either be
the null pointer or point to an object of type struct _roms (which may be
the first of several such contiguous objects, of course).

 

[Mark Richards]

Allin,

> Stop right there. Unless there's some slippage in your quotation
> from the original C source, this is an error. "sensor_list" was
> of type "pointer to struct _roms". The thing you get by writing
> "&sensor_list" is then of type "pointer to pointer to struct _roms",
> or struct _roms **. And this is not compatible with the stated
> parameter type for read_all().

Here's how it's initialized (I've removed extraneous code):

int main(int argc, char *argv[])
{
...
struct _roms sensor_list; /* Attached Roms */
...

if(read_rcfile(conf_file, &sensor_list) < 0);
...
// later on in main():

read_all(&sensor_list);




int read_rcfile(char *fname, struct _roms *sensor_list)
...
sensor_list->roms = malloc(sensors * 8);
sensor_list->max = sensors;

for(x = 0; x < 8; x++)
{
ptr = strtok(NULL, " \t\n");
sensor_list->roms[(sensors * 8) + x] = strtol(ptr, (char **) NULL, 0);
}

-------------------

Indeed, the call read_all(&sensor_list); is being made.

So...

This:
struct _roms sensor_list; /* Attached Roms */

initializes the variable sensor_list which, when referenced, is a
pointer to the start of the structure...

and this:
if(read_rcfile(conf_file, &sensor_list) < 0);

passes that pointer (of sensor_list) to read_rcfile. If instead it read
sensor_list, then the variable would be passed by value, correct?

and this:

int read_rcfile(char *fname, struct _roms *sensor_list)

says that the incoming variable *sensor_list is the address of the
variable sensor_list?

and this:

read_all(&sensor_list);

says to pass the address of sensor_list (not a pointer address) to read_all?



I'm trying to follow the state of sensor_list from initialization. Here
are my assumptions:


struct _roms sensor_list; -> a block of memory defined by the
width of _roms is allocated and
sensor_list is assigned to reference
it;

if(read_rcfile(conf_file, &sensor_list) < 0);
-> the address of sensor_list is passed
into read_rcfile. Oops. I thought
sensor_list was the address;

int read_rcfile(char *fname, struct _roms *sensor_list);
-> sensor_list is now a pointer to the
new structure;

sensor_list->roms
-> the use of sensor_list is now
assumed to be a pointer, since the
function typedef was *sensor_list.
I assume that referencing it like
this would be illegal:

struct _roms sensor_list;
sensor_list->roms (...

It must be done thusly?
*sensor_list->roms (...

Now, back in main():

read_all(&sensor_list);
-> So the address of the address is
being stated here? That does seem
strange. However, this is the
original unmodified code, that works.



As my wife is loathe to say, I'm clearly missing a fundamental here.


-m-

 

[CBFalconer]

When you say it's reserved for the implementation, do you mean
that it's possibly a reserved word, or that the underscore should
not be used?

Please do not toppost. Please do snip non-germane quoted portions.

It is not a reserved word - those are completely specified in the
standard. However the implementation may use identifiers with
leading underscores freely, and is quite likely to do so in the
system headers. The way to avoid clashes is to avoid using such
identifiers.

 

[Mark Richards]

Please do not toppost. Please do snip non-germane quoted portions.
Sorry. Will do.

> The way to avoid clashes is to avoid using such
> identifiers.

Thanks for the clarification.

-m-

 

[Allin Cottrell]

Allin,

Stop right there. Unless there's some slippage in your quotation
from the original C source, this is an error. "sensor_list" was
of type "pointer to struct _roms". The thing you get by writing
"&sensor_list" is then of type "pointer to pointer to struct _roms",
or struct _roms **. And this is not compatible with the stated
parameter type for read_all().

Here's how it's initialized (I've removed extraneous code):

int main(int argc, char *argv[])
{
...
struct _roms sensor_list; /* Attached Roms */

Ah, we're OK then: "sensor_list" is an actual instance of struct
_roms, not a pointer to same. So when you take the address:

&sensor_list

you get something of type "struct _roms *", or pointer-to-struct-
_roms, which is an acceptable parameter to the function read_all(),
cited earlier.

So...

This:
struct _roms sensor_list; /* Attached Roms */

initializes the variable sensor_list which, when referenced, is a
pointer to the start of the structure...

....which, when one takes its address, yields a pointer to the
structure ("start of" is kinda redundant).

and this:
if(read_rcfile(conf_file, &sensor_list) < 0);

passes that pointer (of sensor_list) to read_rcfile. If instead it read
sensor_list, then the variable would be passed by value, correct?

The function read_rcfile() would in that case get a "local copy" of
the structure, and the struct in the caller would remain unmodified,
contrary to the programmer's intention, Yes.

and this:

int read_rcfile(char *fname, struct _roms *sensor_list)

says that the incoming variable *sensor_list is the address of the
variable sensor_list?
Yup.

and this:

read_all(&sensor_list);

says to pass the address of sensor_list (not a pointer address) to
read_all?

The address of a struct (and hence a pointer-to-struct), but not the
address of a pointer, no.

I'm trying to follow the state of sensor_list from initialization. Here
are my assumptions:

struct _roms sensor_list; -> a block of memory defined by the
width of _roms is allocated and
sensor_list is assigned to reference
it;
Yes.

if(read_rcfile(conf_file, &sensor_list) < 0);
-> the address of sensor_list is passed
into read_rcfile. Oops. I thought
sensor_list was the address;

Your current thought is correct.

int read_rcfile(char *fname, struct _roms *sensor_list);
-> sensor_list is now a pointer to the
new structure;

Within the function read_rcfile(), sensor_list figures as
a pointer-to-struct, not a struct "in person", yes.

sensor_list->roms
-> the use of sensor_list is now
assumed to be a pointer, since the
function typedef was *sensor_list.

"typedef" is not the term you want here. The parameter was
declared to be of type "pointer to struct _roms".

I assume that referencing it like
this would be illegal:

struct _roms sensor_list;
sensor_list->roms (...

Yes, if the parameter to the function were of type struct _roms,
you would get a compiler error on using the syntax

sensor_list->roms ...

It must be done thusly?
*sensor_list->roms (...

Huh? If the parameter (counter-factually) had been,
struct _roms sensor_list, then you'd do things like

sensor_list.roms = ...

(In your example, the prepended "*" doesn't make the invalid
"->" any less invalid.)

Now, back in main():

read_all(&sensor_list);
-> So the address of the address is
being stated here?

No, I fear you have lost the place. Up above, you had "sensor_list"
as of type plain "struct _roms" in main.

Allin Cottrell
Wake Forest University

 

[Nick Keighley]

[...]My questions center around those mysterious pointer beasties.

find a good book. "The C Programming Language" by Kernighan and Richie
explains pointers quite well.

// The following _roms structure contains a list of 8 byte addresses...

as others have noted, no it doesn't.

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

// A variable, sensor_list is initialized...

struct _roms *sensor_list

else-thread you change this to

struct _roms sensor_list;

read_all(&sensor_list);

// Above, read_all(&sensor_list) says "call the read_all function and
// pass as a parameter the (starting) address where the variable
// sensor_list is. Is this a correct reading?

call the read_all function with a pointer to the sensor_list struct.
If you like you use "address" instead of "pointer". On a typical
implementation pointers will be addresses.

int read_all(struct _roms *sensor_list)
{
int x;
for(x = 0; x < (num_cs + sensor_list->max); x++)
{
read_device(sensor_list, x);
}
return 0;
}

// It appears that a de-referenced pointer to sensor_list is being
// defined in the function header, correct?

no. sensor_list is a pointer to struct _roms. This is the source of
confusion to beginning C programmers. * is used in two different ways
(well three if you count multiplication).

int *a;

a is ptr to int

*a = 1;

a is being dereferenced. a is still a pointer to int but *a is an int.
Another way of looking at it that

int *a;

declares *a to be an int. Usage mirrors declaration syntax. Opinions vary
as to whether this was a good idea.

// Within read_device, the sensor_list structure is used to pass a
// complete 8-byte address to a function called owSerialNum:

owSerialNum( 0, &sensor_list->roms[sensor*8], FALSE );

why did you multiply by 8?
why did you take the address?
Try:-

owSerialNum (0, sensor_list->roms [sensor], FALSE;

(assuming you've initialised roms correctly)

// If I call the function like this:

owSerialNum( 0, "12BAA91C000000A3", FALSE );

// it works perfectly. The documentation for owSerialNum says that the
// second parameter is a buffer containing the address.

// I can see that &sensor_list->roms[sensor*8] is saying, "the address
// of roms[beginning at this offset] within a structure we call
// "sensor_list".

yes, but roms is a pointer to unsigned char, why take its address?

// Since the second parameter in
// owSerialNum( 0,"12BAA91C000000A3",FALSE); is passed through as the
// starting address of a buffer, and since &sensor_list->roms[sensor*8]
// is also the address of a buffer, they must be synonymous, correct?

nope.

<snip>


--
Nick Keighley

You are in a clearing. You can see a spire in the distance.
You can also see a copy of "C Unleashed".
: INV
You have;
a DeathStation 900 laptop,
a voucher for a free pizza,
and a torch.
: TAKE BOOK
You can't. It's too heavy.
Bill Godfrey (clc)

 

[Mark Richards]

Nick,

Thanks for your book suggestion. I have it, and have read the pointers
and arrays chapter once more. Something, however, isn't sinking in (yet).

I think there was some confusion in my use of the term "address".

In the structure:

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

*roms provides storage for a "8-byte 1-wire device address". This
memory location is supposed to store a string similar to
"12BAA91C000000A3". I should have been very specific with my use of
this term.

I've read carefully what you conveyed. I cannot understand the
effective difference between:

owSerialNum( 0, "12BAA91C000000A3", FALSE );

and
int sensor = 0;
owSerialNum( 0, &sensor_list->roms[sensor*8], FALSE );


owSerialNum( 0, "12BAA91C000000A3", FALSE ); and owSerialNum( 0,
&sensor_list->roms[sensor*8], FALSE ); both work properly, but if I:

unsigned char TempSN[16];
strcpy(TempSN,"12BAA91C000000A3");
printf("Rom address: %s",TempSN);

I get: "Rom address: 12BAA91C000000A3"

If I do this:

sensor = 0;
strcpy(TempSN,&sensor_list.roms[sensor*8]);
printf("Rom address: %s",TempSN);

I get: "Rom address: ;X" (garbage). In the above, I don't quite see why
&sensor_list.roms[sensor*8] doesn't return a string since the
dereference operator is used.


If I take your suggestion and remove the address of operator (&):

owSerialNum (0, sensor_list->roms [sensor], FALSE);

I get a compiler warning: passing arg 2 of `owSerialNum' makes pointer
from integer without a cast.

>else-thread you change this to
>
> struct _roms sensor_list;

I was not being specific enough here, too.

In main() it's defined thusly:
struct _roms sensor_list; /* Attached Roms */

It's passed in a function call like this:

int read_all(struct _roms *sensor_list)

And within the function that it is passed as a parameter, it's used like
this:

read_all(&sensor_list);



So, based on your explanation, is it correct to say:

struct _roms sensor_list;

defines the variable sensor_list which is yet to be initialized;

int read_all(struct _roms *sensor_list)

received a pointer (the address of) sensor_list;

and
read_all(&sensor_list);

passes the value found at sensor_list's address?


.....


As I continued to puzzle at sensor_list, I tried the following code:

struct _roms sensor_list;
struct _roms *new_sensor_list;
unsigned char *theaddress;

/* Make sure the structure is erased */
bzero(&sensor_list, sizeof(struct _roms));

// sensor_list.roms is NULL now.

// this initializes sensor_list
read_rcfile(conf_file, &sensor_list);
// sensor_list.roms is NOT NULL now.

// initialize pointer
new_sensor_list = &sensor_list;
theaddress = &new_sensor_list->roms[0];

printf("Rom address: %s",theaddress);

This outputs garbage.

 

[Allin Cottrell]

If I do this:

sensor = 0;
strcpy(TempSN,&sensor_list.roms[sensor*8]);
printf("Rom address: %s",TempSN);

I get: "Rom address: ;X" (garbage). In the above, I don't quite see why
&sensor_list.roms[sensor*8] doesn't return a string since the
dereference operator is used.

The character '*' does double duty in C: as the multiplication operator,
and for dereferencing a pointer. In well-formed code the specific
meaning is unambiguous in context. In the fragment above, since both
'sensor' and '8' represent integers, the '*' is unambiguously the
multiplication operator: sensor*8 = 0*8 = 0.

Allin Cottrell

 

[CBFalconer]

Thanks for your book suggestion. I have it, and have read the
pointers and arrays chapter once more. Something, however, isn't
sinking in (yet).

I think there was some confusion in my use of the term "address".

In the structure:

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

*roms provides storage for a "8-byte 1-wire device address".
This memory location is supposed to store a string similar to
"12BAA91C000000A3". I should have been very specific with my
use of this term.

As you have been told before, no it doesn't. The comment is
WRONG. roms provides space for a pointer to char, and is
uninitialized.

Why ask for advice if you simply ignore it?

 

[Barry Schwarz]

Nick,

Thanks for your book suggestion. I have it, and have read the pointers
and arrays chapter once more. Something, however, isn't sinking in (yet).

I think there was some confusion in my use of the term "address".

In the structure:

struct _roms
{
unsigned char *roms; /* Array of 8 bytes */
int max; /* Maximum number */
};

*roms provides storage for a "8-byte 1-wire device address". This
memory location is supposed to store a string similar to
"12BAA91C000000A3". I should have been very specific with my use of
this term.

You are having trouble with both terminology and notation.

Until you initialize it with some valid address, roms does not provide
anything other than the potential to be initialized.

Once roms is initialized and once the area it points is initialized,
*roms is always exactly the value of the first character (or byte if
you prefer) in that area.

You say it is the address of an 8 byte area but then you give an
example of a 17 byte string. Did you perhaps mean the 8 hex values
0x12, 0xba, 0xa9, ...?

I've read carefully what you conveyed. I cannot understand the
effective difference between:

owSerialNum( 0, "12BAA91C000000A3", FALSE );

and
int sensor = 0;
owSerialNum( 0, &sensor_list->roms[sensor*8], FALSE );

If roms is not initialized, the second invokes undefined behavior. If
roms is initialized to point to an 8-byte area as you suggest, then
the two cannot be equivalent since

The first passes a string of length 16 and each character (except
the 17th which is '\0') is printable.

The second passes an array of only 8 characters (per your comment)
which are most likely unprintable and probably not nul terminated.

owSerialNum( 0, "12BAA91C000000A3", FALSE ); and owSerialNum( 0,
&sensor_list->roms[sensor*8], FALSE ); both work properly, but if I:

unsigned char TempSN[16];
strcpy(TempSN,"12BAA91C000000A3");

This invokes undefined behavior since strcpy will attempt to copy the
'\0' and there is no room in TempSN for it.

printf("Rom address: %s",TempSN);

I get: "Rom address: 12BAA91C000000A3"

If I do this:

sensor = 0;
strcpy(TempSN,&sensor_list.roms[sensor*8]);
printf("Rom address: %s",TempSN);

I get: "Rom address: ;X" (garbage). In the above, I don't quite see why
&sensor_list.roms[sensor*8] doesn't return a string since the

You need to show how roms is initialized and how the area it points to
is initialized.

dereference operator is used.


If I take your suggestion and remove the address of operator (&):

owSerialNum (0, sensor_list->roms [sensor], FALSE);

I get a compiler warning: passing arg 2 of `owSerialNum' makes pointer
from integer without a cast.

As well you should. If you remove the &, you should also remove the
[sensor].

I was not being specific enough here, too.

In main() it's defined thusly:
struct _roms sensor_list; /* Attached Roms */

It's passed in a function call like this:

int read_all(struct _roms *sensor_list)

And within the function that it is passed as a parameter, it's used like
this:

read_all(&sensor_list);



So, based on your explanation, is it correct to say:

struct _roms sensor_list;

defines the variable sensor_list which is yet to be initialized;
Yes


int read_all(struct _roms *sensor_list)

received a pointer (the address of) sensor_list;
Yes


and
read_all(&sensor_list);

passes the value found at sensor_list's address?

No. It passes the address itself. It cares not a whit for any value
or missing value at that address.

....


As I continued to puzzle at sensor_list, I tried the following code:

struct _roms sensor_list;
struct _roms *new_sensor_list;
unsigned char *theaddress;

/* Make sure the structure is erased */
bzero(&sensor_list, sizeof(struct _roms));

// sensor_list.roms is NULL now.

No, sensor_list.roms is now all bits 0 and you have no idea what that
value means in a char*.

// this initializes sensor_list
read_rcfile(conf_file, &sensor_list);
// sensor_list.roms is NOT NULL now.

// initialize pointer
new_sensor_list = &sensor_list;
theaddress = &new_sensor_list->roms[0];

printf("Rom address: %s",theaddress);

While is may be true that roms is not NULL after the return from
read_rcfile, how was the area it points to initialized? In
particular, was it initialized with a printable nul-terminated string?

This outputs garbage.

You need to post some compilable code that demonstrates the problem.


<<Remove the del for email>>

 

[tristan]

Here's how it's initialized (I've removed extraneous code):

int main(int argc, char *argv[])
{
...
struct _roms sensor_list; /* Attached Roms */

[big snip]

Yup.

Maybe. Note that, syntactically speaking, the thing named sensor_list
in this function bears no relation whatsoever to the thing in main()
named sensor_list. read_rcfile() knows only that it has a local
variable called sensor_list, which is a pointer to a struct _roms.

The address of a struct (and hence a pointer-to-struct), but not the
address of a pointer, no.


Yes.

You now have a local variable of type struct _roms, which is known
by the name sensor_list. It isn't allocated in the sense of
dynamic allocation, nor is sensor_list a reference in the sense
that e.g. Java uses it.

Your current thought is correct.

sensor_list is the variable itself.

Within the function read_rcfile(), sensor_list figures as
a pointer-to-struct, not a struct "in person", yes.

Which, apart from having an identical spelling, is completely
separate from the previously mentioned uses of sensor_list.

"typedef" is not the term you want here. The parameter was
declared to be of type "pointer to struct _roms".

And there's no need to assume, it's all quite explicit

[small snip]

No, I fear you have lost the place. Up above, you had "sensor_list"
as of type plain "struct _roms" in main.

I don't think you mentioned what language(s) you had previously
programmed in. Be aware that in C, variables are always passed by
value, and names of variables are direct aliases for values. If you
want an address, you must be explicit. This includes variables of
struct types. This is in contrast to something like Java, where almost
all variables are actually indirect references, without you ever saying
so specifically.

HTH
Tristan

 

[Mark Richards]

I'm so greatful for all the help, encouragement, and kicks in the arse
that I've received here. As a result, I tried to apply all the good
advice, and worked through the problem as carefully as my ability and
experience has enabled. It was especially helpful to try and build a
test application which duplicates the functions in the application I'm
attempting to understand. I found that this is a really great way to
learn and breaks things down into understandable segments.

I'd really appreciate it if you might have a look at this code and offer
up criticisms, or reiterate those that I unintentionally overlooked.
I'm certain that better minds than mine will have a few.

Thanks!


(To generate the executable:
gcc -o testapp testapp.c && ./testapp && rm -f testapp.o
Save the source as testapp.c)



// BEGIN testapp.c

//This generates a small file called file.txt. file.txt is a replica of
//a segment of the file that the larger application uses to store
//various data, including the ROM address.

#include <strings.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

struct _roms
{
unsigned char *roms;
int max;
};


int initlist(struct _roms *sensor_list)
{

int sensors;
sensors = 1;

sensor_list->roms = malloc( sensors * 8 );
sensor_list->max = sensors;

return 0;
}

int loadstring(char *fname, struct _roms *sensor_list)
{
int x;
char astring[17];
char *ptrstring;
char *ptr;
int sensors=0;
char temp[1024];
FILE *fp;


// create the file that contains the string address
if( ( fp = fopen( fname, "w" ) ) == NULL )
{
printf("Error creating file\n");
return 1;
}

// write string address to file
if (fputs("0x12 0xBA 0xA9 0x1C 0x00 0x00 0x00 0xA3",fp) == 0)
{
printf("Error writing file\n");
return 1;
}
fclose(fp);

// re-open file and read contents into temp
if( ( fp = fopen( fname, "r" ) ) == NULL )
{
printf("Error opening file\n");
return 1;
}

while( fgets( temp, 80, fp ) != 0 )
{

if( (temp[0] == '\n') || (temp[0] == '#') ) continue;
ptr = strtok(temp, " \t\n");

sensors = 0;
for(x = 0; x < 8; x++)
{
sensor_list->roms[(sensors * 8) + x] = strtol(ptr, (char **) NULL,0);
ptr = strtok(NULL, " \t\n");
}
sensor_list->max = 1;
}
fclose(fp);

return 0;
}


int printit(struct _roms *sensor_list)
{
unsigned char *ptr;
int x,y;
unsigned char cstring[17];


for( x = 0; x < sensor_list->max; x++ )
{
printf("ROM %d = ", x );

for( y = 0; y < 8; y++ )
{
printf( "0x%02X ", sensor_list->roms[(x * 8) + y] );
}
printf( "\n" );
}

for( x = 0; x < sensor_list->max; x++ )
{
printf("ROM %d = ", x );

for( y = 0; y < 8; y++ )
{
printf( "%d ", sensor_list->roms[(x * 8) + y] );
}
printf( "\n" );
}


return 0;
}


int main(int argc, char *argv[])
{

// *roms is an array of 8 bytes
// max is the maximum number of entries
struct _roms sensor_list;

int sensors;
int x;
char conf_file[1024];
char *ptr;

printf("Start of test application\n");

strcpy(conf_file, "file.txt");

bzero(&sensor_list, sizeof(struct _roms));

x = initlist(&sensor_list);
x = loadstring(conf_file,&sensor_list);
x = printit(&sensor_list);

printf("End of test application\n");
return 0;
}

// END testapp.c

 

[Allin Cottrell]

// BEGIN testapp.c

//This generates a small file called file.txt. file.txt is a replica of
//a segment of the file that the larger application uses to store
//various data, including the ROM address.

#include <strings.h>

non-standard header: dump it.

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

struct _roms
{
unsigned char *roms;
int max;
};


int initlist(struct _roms *sensor_list)
{

int sensors;
sensors = 1;

sensor_list->roms = malloc( sensors * 8 );
sensor_list->max = sensors;

return 0;
}

You'd be better off passing in "sensors" (the number of sensors, which
I would suggest renaming as "n_sensors") as a parameter to this
function. You should also check that malloc didn't return NULL
(and return an error code to the caller if it did).

int loadstring(char *fname, struct _roms *sensor_list)
{
int x;
char astring[17];
char *ptrstring;

Unused variables astring, ptrstring.

char *ptr;
int sensors=0;
char temp[1024];
FILE *fp;


// create the file that contains the string address
if( ( fp = fopen( fname, "w" ) ) == NULL )
{
printf("Error creating file\n");
return 1;
}

// write string address to file
if (fputs("0x12 0xBA 0xA9 0x1C 0x00 0x00 0x00 0xA3",fp) == 0)

Better to put '\n' to terminate the line.

{
printf("Error writing file\n");
return 1;
}
fclose(fp);

// re-open file and read contents into temp
if( ( fp = fopen( fname, "r" ) ) == NULL )
{
printf("Error opening file\n");
return 1;
}

while( fgets( temp, 80, fp ) != 0 )

plain "while (fgets(temp, 80, fp))" is more idiomatic. Or
"while (fgets(temp, sizeof temp, fp))" to avoid magic numbers.

{

if( (temp[0] == '\n') || (temp[0] == '#') ) continue;
ptr = strtok(temp, " \t\n");

strtok is deprecated hereabouts, because it messes up the original
string (sticks in '\0' characters). Better to use strcspn or
(better still) sscanf to pick out the numeric fields in the line.

sensors = 0;
for(x = 0; x < 8; x++)
{
sensor_list->roms[(sensors * 8) + x] = strtol(ptr, (char **) NULL,0);

strtol is regarded as "better" than plain atoi on account of the error-
checking it affords. But you're doing no error checking here, so atoi
would do the same job. As mentioned above, sscanf would be better
anyway (check that you get 8 valid ints in the line).

ptr = strtok(NULL, " \t\n");
}
sensor_list->max = 1;

You mean, sensor_list->max += 1 (with the value having been initialized
to zero prior to reading)?

}
fclose(fp);

return 0;
}


int printit(struct _roms *sensor_list)
{
unsigned char *ptr;
int x,y;
unsigned char cstring[17];

unused variables ptr and cstring.

for( x = 0; x < sensor_list->max; x++ )
{
printf("ROM %d = ", x );

for( y = 0; y < 8; y++ )
{
printf( "0x%02X ", sensor_list->roms[(x * 8) + y] );
}
printf( "\n" );
}

for( x = 0; x < sensor_list->max; x++ )
{
printf("ROM %d = ", x );

for( y = 0; y < 8; y++ )
{
printf( "%d ", sensor_list->roms[(x * 8) + y] );
}
printf( "\n" );
}


return 0;
}


int main(int argc, char *argv[])
{

// *roms is an array of 8 bytes
// max is the maximum number of entries
struct _roms sensor_list;

int sensors;
int x;
char conf_file[1024];
char *ptr;

Unused variable ptr.

printf("Start of test application\n");

strcpy(conf_file, "file.txt");

bzero(&sensor_list, sizeof(struct _roms));

Don't use the non-standard bzero. Use an intializer:

struct _roms sensor_list = {0};

x = initlist(&sensor_list);

Rename "x" to "err" (or similar), and react appropriately
if one of the functions returns a non-zero value.

 

[Barry Schwarz]

I'm so greatful for all the help, encouragement, and kicks in the arse
that I've received here. As a result, I tried to apply all the good
advice, and worked through the problem as carefully as my ability and
experience has enabled. It was especially helpful to try and build a
test application which duplicates the functions in the application I'm
attempting to understand. I found that this is a really great way to
learn and breaks things down into understandable segments.

I'd really appreciate it if you might have a look at this code and offer
up criticisms, or reiterate those that I unintentionally overlooked.
I'm certain that better minds than mine will have a few.

Thanks!


(To generate the executable:
gcc -o testapp testapp.c && ./testapp && rm -f testapp.o
Save the source as testapp.c)



// BEGIN testapp.c

//This generates a small file called file.txt. file.txt is a replica of
//a segment of the file that the larger application uses to store
//various data, including the ROM address.

#include <strings.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>

struct _roms
{
unsigned char *roms;
int max;
};


int initlist(struct _roms *sensor_list)

There doesn't seem to be any point to the return value of this
function. It is always 0.

{

int sensors;
sensors = 1;

sensor_list->roms = malloc( sensors * 8 );
sensor_list->max = sensors;

return 0;
}

int loadstring(char *fname, struct _roms *sensor_list)
{
int x;
char astring[17];
char *ptrstring;
char *ptr;
int sensors=0;
char temp[1024];
FILE *fp;


// create the file that contains the string address
if( ( fp = fopen( fname, "w" ) ) == NULL )
{
printf("Error creating file\n");
return 1;
}

// write string address to file
if (fputs("0x12 0xBA 0xA9 0x1C 0x00 0x00 0x00 0xA3",fp) == 0)

fputs returns EOF on error. A return of 0, or any other non-negative
value, indicates success.

{
printf("Error writing file\n");
return 1;
}
fclose(fp);

// re-open file and read contents into temp
if( ( fp = fopen( fname, "r" ) ) == NULL )
{
printf("Error opening file\n");
return 1;
}

while( fgets( temp, 80, fp ) != 0 )

fgets returns a pointer. While comparing a pointer to 0 is legal, it
is stylistically more informative to use NULL.

You wrote exactly 40 characters to the file. The data you wrote did
not include a '\n'. Telling fgets to read 80 characters is guaranteed
to cause end of file to occur. This will cause fgets to return NULL
and the while expression can never evaluate to true.

{

if( (temp[0] == '\n') || (temp[0] == '#') ) continue;

These characters do not appear in your sample string.

ptr = strtok(temp, " \t\n");

Only the space appears in your sample.

sensors = 0;
for(x = 0; x < 8; x++)
{
sensor_list->roms[(sensors * 8) + x] = strtol(ptr, (char **) NULL,0);

The cast is superfluous. Since there is a prototype in scope, the
compiler will automatically convert NULL from its defined type to
char**.

ptr = strtok(NULL, " \t\n");
}
sensor_list->max = 1;
}
fclose(fp);

return 0;
}


int printit(struct _roms *sensor_list)

There doesn't seem to be any point to the return value of this
function. It is always 0.

{
unsigned char *ptr;
int x,y;
unsigned char cstring[17];


for( x = 0; x < sensor_list->max; x++ )
{
printf("ROM %d = ", x );

for( y = 0; y < 8; y++ )
{
printf( "0x%02X ", sensor_list->roms[(x * 8) + y] );
}
printf( "\n" );
}

for( x = 0; x < sensor_list->max; x++ )
{
printf("ROM %d = ", x );

for( y = 0; y < 8; y++ )
{
printf( "%d ", sensor_list->roms[(x * 8) + y] );
}
printf( "\n" );
}


return 0;
}


int main(int argc, char *argv[])

Since you don't use either parameter, you can get rid of an annoying
diagnostic that many compilers produce by using
int main(void)

{

// *roms is an array of 8 bytes

As several of us have already stated, neither roms nor *roms is an
array.

// max is the maximum number of entries
struct _roms sensor_list;

int sensors;
int x;
char conf_file[1024];
char *ptr;

printf("Start of test application\n");

strcpy(conf_file, "file.txt");

bzero(&sensor_list, sizeof(struct _roms));

This does not do anything useful and the resulting value in
sensor_list.roms may not be legal. Of course, it's also not a
standard function so maybe it does do something useful.

x = initlist(&sensor_list);

Since you don't use the return value, there doesn't seem to be any
point in assigning it to x.

x = loadstring(conf_file,&sensor_list);

You should be checking this for success.

x = printit(&sensor_list);

printf("End of test application\n");
return 0;
}

// END testapp.c

<<Remove the del for email>>

 

[Al Bowers]

Barry Schwarz wrote:

You wrote exactly 40 characters to the file. The data you wrote did
not include a '\n'. Telling fgets to read 80 characters is guaranteed
to cause end of file to occur. This will cause fgets to return NULL
and the while expression can never evaluate to true.

That is not correct. The Standard says

char *fgets(char * restrict s, int n,
FILE * restrict stream);


The fgets function returns s if successful. If end-of-file is
encountered and NO characters have been read into the array,
^^
the contents of the array remain unchanged and a null pointer
is returned.

 

[Barry Schwarz]

Barry Schwarz wrote:



That is not correct. The Standard says

char *fgets(char * restrict s, int n,
FILE * restrict stream);


The fgets function returns s if successful. If end-of-file is
encountered and NO characters have been read into the array,
^^
the contents of the array remain unchanged and a null pointer
is returned.

I should have checked the standard rather than depend on the brief
description in K&R, pg 247.


<<Remove the del for email>>