writing library functions and pointers?

[Martin]

Hi,

I have been trying to construct my own library of commonly used C
functions and have got a bit stuck and wonder if anyone could give me
any pointers?

I have set up a generic read function...

void read_file_float(FILE **fp, float **array, int size, char **argv)
{
if (fread(*array, sizeof (float), size, *fp) != size) {
fprintf(stderr, "Error reading in file: %s\n", *argv);
exit(1);
}
return;
}

which I call using the following in my code for example

FILE *fp;
float *array (which I allocate);
int numrows = something;
int numcols = something

read_file_float(&fp, &array, numrows * numcols, argv);

This works fine.

The problem comes when I do something similar but instead use fseek -
so in this instance i only want to read part of the file into the
array e.g. array[counter]

Now my call to the function would be

read_file_float(&fp, &array[some_counter]), 1, argv);

But I don't know how to declare this correctly.

Any thoughts?
Thanks in advance

 

[Martin]

I think I have solved it.

function needs to be declared differently

void read_seg_float(FILE **fp, float *array, int size, char **argv)
{
if (fread(array, sizeof (float), size, *fp) != size) {
fprintf(stderr, "Error reading in file: %s\n", *argv);
exit(1);
}
return;
}

and then accessed

read_seg_float(&fp, &array[some_counter], size, argv);

 

[Nobody]

I have been trying to construct my own library of commonly used C
functions and have got a bit stuck and wonder if anyone could give me
any pointers?

I have set up a generic read function...

void read_file_float(FILE **fp, float **array, int size, char **argv)

Why are you passing a FILE** rather than just a FILE*? And a float**
rather than just a float*?

 

[Martin]

Why are you passing a FILE** rather than just a FILE*? And a float**
rather than just a float*?

because I understood the way adjust passed copy of pointer was to use
a pointer to a pointer. I got that from the FAQ, perhaps I
misunderstood?

http://c-faq.com/ptrs/passptrinit.html

 

[Stephen Sprunk]

because I understood the way adjust passed copy of pointer was to use
a pointer to a pointer. I got that from the FAQ, perhaps I
misunderstood?

http://c-faq.com/ptrs/passptrinit.html

You're not changing the caller's pointer, only the object that the
pointer is pointing to.

S

 

[Barry Schwarz]

I think I have solved it.

function needs to be declared differently

void read_seg_float(FILE **fp, float *array, int size, char **argv)
{
if (fread(array, sizeof (float), size, *fp) != size) {
fprintf(stderr, "Error reading in file: %s\n", *argv);
exit(1);
}
return;
}

and then accessed

read_seg_float(&fp, &array[some_counter], size, argv);

If you had compiled this with your calling function, you would see
that it is not correct. In the calling function, array is declared as
array of float*. Therefore, array[some_counter] is a particular
float*. Therefore, &array[some_counter] has type float**. Since the
second parameter in the definition of read_seg_float is a float*, this
is a constraint violation requiring a diagnostic. The compiler cannot
generate correct code for this even if it converts the value
&array[some_counter] to type float*.

 

[Barry Schwarz]

Hi,

I have been trying to construct my own library of commonly used C
functions and have got a bit stuck and wonder if anyone could give me
any pointers?

I have set up a generic read function...

void read_file_float(FILE **fp, float **array, int size, char **argv)
{
if (fread(*array, sizeof (float), size, *fp) != size) {
fprintf(stderr, "Error reading in file: %s\n", *argv);
exit(1);
}
return;
}

which I call using the following in my code for example

FILE *fp;
float *array (which I allocate);
int numrows = something;
int numcols = something

read_file_float(&fp, &array, numrows * numcols, argv);

The unnecessary & applied to fp is discussed elsethread.

array is an object. A such, it is located in memory and has an
address. It also contains a value. Since it is a pointer object,
that value happens to be the address of an area of memory suitable for
holding a number of floats. &array evaluates to the address of the
object.

In your function, you immediately dereference this value. The result
of this operation is the contents of the pointer object which is the
location where you want to store data. Note that nowhere in your
function do you attempt to modify the contents of the pointer object
itself. Consequently, there is no need to pass the address of the
pointer to the function. Thus, the & applied to array is just as
unnecessary as the one applied to fp.

To answer your response in a subsequent message, yes you have
misunderstood slightly. Every argument passed to a function is passed
by value (as if a copy of the argument is passed to the function).
This is true whether the object is a number (integer or floating) or
an address. As a result of this, when the argument is a scalar
object, the function is incapable of modifying the value of the object
***in the calling function***. It is capable of modifying the local
copy that was passed but this is not the same as the original object.

Therefore, when the function needs to modify the object in the calling
function, one common technique is to pass the address of the object
(using the & operator). The function can then dereference this
address to get access to the object in the calling function.

However, this does not apply to your function because it us not
attempting to modify any of the arguments it is passed. fread never
updates the FILE* you pass it; it only updates the FILE the pointer
points to. Your code does not attempt to modify either copy of array,
only the memory that array in the calling function points to. While
the code you have will work, because you added the necessary
dereference operators, it is unnecessary. You can remove the &'s from
the calling statement and remove one * from every use of fp and array.

This works fine.

The problem comes when I do something similar but instead use fseek -
so in this instance i only want to read part of the file into the
array e.g. array[counter]

Now my call to the function would be

read_file_float(&fp, &array[some_counter]), 1, argv);

But I don't know how to declare this correctly.

If you fix your function to expect a simple float* as described above,
this will magically be correct. If for some reason you have not
disclosed to us you really need the extra level of indirection, then
one method for reading a single value would be
float *x;
x = &array[some_counter];
read_file_float(&fp, &x, 1, argv);

 

[Martin]

Hi thanks for all of the replies...

yes I guess a combination of me misunderstanding and because I
attempted to adapt the first function I wrote to allocate memory:

float *array;
int size = some_number;

void allocate_memory_float(float **array, int size, const unsigned int
line)
{
if (!(*array = (float *)calloc((size + 1), sizeof (float)))) {
check_errors("Error allocating enough memory for array", line);
}
return;
}

which I call:

allocate_memory_float(&array, size, __LINE__);

Which I think in this case as I am changing the array it is fine to
pass it like this? Just not for fread because I am not actually
changing anything?

Thanks again, the help really is appreciated.

 

[Barry Schwarz]

Hi thanks for all of the replies...

yes I guess a combination of me misunderstanding and because I
attempted to adapt the first function I wrote to allocate memory:

float *array;
int size = some_number;

void allocate_memory_float(float **array, int size, const unsigned int
line)
{
if (!(*array = (float *)calloc((size + 1), sizeof (float)))) {

You don't need to cast the return from calloc. In fact, doing so can
cause the compiler to fail to warn you about some undefined behavior.

You also don't need the parentheses around size+1.

While using calloc does not cause any problems, it also doesn't
provide a portable benefit. It WILL set every byte of the allocated
memory to all-bits-zero. However, there is NO guarantee this
represents 0.0f or any other valid float value.

By the way, for messages posted to Usenet it is better to use spaces
than tabs to perform indenting. It eliminates problems due the
variety of newsreaders used.

check_errors("Error allocating enough memory for array", line);
}
return;
}

which I call:

allocate_memory_float(&array, size, __LINE__);

Which I think in this case as I am changing the array it is fine to
pass it like this? Just not for fread because I am not actually
changing anything?

While this approach will work, the more common C idiom would be

#include <stdlib.h>
float* allocate_memory_float(int size, const unsigned int line)
{
float *ptr;
if (!(ptr = malloc((size + 1) * sizeof *ptr))) {
check_errors("Error allocating enough memory for array",
line);
}
return ptr;
}

which is called with
array = allocate_memory_float(size, __LINE__);

If you were willing to do the error checking in the calling function,
the entire body of allocate_memory_float could be replaced with
return malloc((size + 1) * sizeof *ptr);
which would basically eliminate the need for a separate function
entirely.

 

[Martin]

Thanks a lot Barry I will adopt the form you suggested instead. Two
more questions if you have a moment

1.Do you have a suggestion for how I would write a function for the
allocation of my structure?

Currently I set a structure up

typedef struct {
int something;
int something_else;
} control;

in my .h file and then I allocate it in my code

control *c;
if ((c = (control *)malloc(sizeof (control))) == NULL) {
check_errors("Structure: Not allocated enough memory", __LINE__);
}

I don't know how I would set this up a library function

2. Also more a general question, currently I set up one big sturcture
and pass this around throughout my code. Do you suggest many smaller
structures would be better in practice? Does it matter? I guess the
later way reduces what is sent through the code, but does this
practically matter?

Thanks again

 

[Morris Keesan]

control *c;
if ((c = (control *)malloc(sizeof (control))) == NULL) {
check_errors("Structure: Not allocated enough memory", __LINE__);
}

I don't know how I would set this up a library function

control *allocate_control(void)
{
control *c;
if ((c = malloc(sizeof (control))) == NULL) {
check_errors("Structure: Not allocated enough memory", __LINE__);
return c;
}

2. Also more a general question, currently I set up one big sturcture
and pass this around throughout my code. Do you suggest many smaller
structures would be better in practice? Does it matter? I guess the
later way reduces what is sent through the code, but does this
practically matter?

A common way of dealing with this is to pass a pointer to the structure,
instead of passing the structure itself. This is usually more efficient
than passing the entire structure. When passing by reference like this,
a useful habit to get into is to use the "const" attribute in the function
declaration, when the function is not intended to modify the contents of
the structure. This has the benefits of
* Giving useful information to the human reader of the code.
* Giving useful information to optimizing compilers
* Generating a compile-time error message if someone tries to
change the implementation of the function in a way which would
modify the contents of the structure.

E.g.
int do_something_with_my_structure (const my_structure *p);

 

[Barry Schwarz]

Thanks a lot Barry I will adopt the form you suggested instead. Two
more questions if you have a moment

1.Do you have a suggestion for how I would write a function for the
allocation of my structure?

Currently I set a structure up

typedef struct {
int something;
int something_else;
} control;

in my .h file and then I allocate it in my code

control *c;
if ((c = (control *)malloc(sizeof (control))) == NULL) {
check_errors("Structure: Not allocated enough memory", __LINE__);
}

I don't know how I would set this up a library function

What is different between this effort and the one you used for array?
Why put it in a function at all? It's just a single statement.

But whatever you do, LOSE THE CAST.

2. Also more a general question, currently I set up one big sturcture
and pass this around throughout my code. Do you suggest many smaller
structures would be better in practice? Does it matter? I guess the
later way reduces what is sent through the code, but does this
practically matter?

As Morris mentioned, one approach is to pass a pointer to the struct
instead of the struct itself. If the function being called should not
change any of the contents of the struct, then define the parameter as
a pointer to const struct.

While several smaller structures will probably consume slightly more
space, if the contents of the structure can be logically
compartmentalized the greater granularity could make coding easier.
For example, if function-1 should update some members of the structure
but not others, if those members are in a separate structure you could
pass only that portion and be assured that a coding error will not
cause function-1 to update an unintended portion.

 

[Morris Keesan]

.... ....

But whatever you do, LOSE THE CAST.

This is a sufficiently common idiom that it's worth explaining WHY
the cast should be removed.

Casting the return value of malloc to the type of the left-hand-side was
never necessary, in any version of C I've ever used. But before the
introduction of the (void *) type, when malloc returned a (char *), some
versions of lint would complain about the assignment, which got people
into the habit of adding the cast to eliminate this spurious warning.

The disadvantage of the cast is that, if you accidentally omit including
a proper declaration for malloc (usually by forgetting to include
<stdlib.h>),
inserting the cast will hide this from you, because a cast is an explicit
directive to the compiler saying that you know what you're doing. Without
the cast, the implicitly (int) return value from malloc is incompatible
with
whatever pointer type is on the left hand side of the assignment operator,
and
the compiler is required to issue an error message, informing you of this.

 

[Martin]

Thanks for all of that explanation, it is appreciated.

Regarding the casting - I guess it was something I adapted from
someone's code and I hadn't really given it a lot of consideration if
I am honest. I am slightly confused after reading the last post
(perhaps this is over my level of C coding). I shall re read the posts
again, but I guess I shall remove the casting for the moment?

I agree the allocation of my structure is only a few lines. But it
occurred to me the other day I am constantly copying and pasting bits
of code I do a lot from one piece of code to another, e.g. allocating
memory for arrays etc. So I remembered what the person who taught me
suggested about make my own libraries and thought I would try and
construct a library of things I commonly do to tidy up my code and
make my working process a bit more efficient. Allocating structures
was also something I found I did a lot, so I thought I would make a
function for this too.

Sorry I also think I wasn't clear with my question regarding
structures. At the moment I allocate one control structure and fill
this up with things I need to pass through the code, i.e. things I
read in from the cmd line, things Im moving btw functions etc and then
pass it around like this...

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

some_function(c);

}

void some function (control *c)
{

}

and so on.

I just wondered if this was bad practice and whether it was better to
allocate lots of structures, or if it even matters. E.g.

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

some_function(c, sp, se);

}
void some function (control *c, some_parameters *sp, something_else
*se)
{

}

where some_parameters and something_else are both structures. Perhaps
the latter way is more readable for others? My code is really only
read by me...but always happy to try and improve its readability

Thanks again!

Martin

 

[Morris Keesan]

Regarding the casting - I guess it was something I adapted from
someone's code and I hadn't really given it a lot of consideration if
I am honest. I am slightly confused after reading the last post
(perhaps this is over my level of C coding). I shall re read the posts
again, but I guess I shall remove the casting for the moment?

Don't be in a hurry to remove the cast, especially not if you don't
really understand why you're doing it. As you've seen, experts disagree
about this. Make your own decisions. People feel strongly in both
directions. Personally, I prefer not to use the cast, but I don't
bother removing it from existing code.

 

[Martin]

Don't be in a hurry to remove the cast, especially not if you don't
really understand why you're doing it.  As you've seen, experts disagree
about this.  Make your own decisions.  People feel strongly in both
directions.  Personally, I prefer not to use the cast, but I don't
bother removing it from existing code.

Ok thanks - I have been researching it now and I think I understand
the issue now. I thought this page explained it well
http://everything2.com/title/Casting%20the%20return%20value%20of%20malloc%28%29
if that helps anyone else.

 

[Eric Sosman]

Ok thanks - I have been researching it now and I think I understand
the issue now. I thought this page explained it well
http://everything2.com/title/Casting%20the%20return%20value%20of%20malloc%28%29
if that helps anyone else.

The treatment on that page is at best superficial, and
contains at least three minor errors:

1) It incorrectly states that the Ten Commandments say to
cast the value of malloc(). This is probably due to an
over-hasty mis-reading of the Third Commandment, but
could possibly be a complete misunderstanding of the
Fourth.

2) It incorrectly claims that a void* can be "converted to
any other pointer type." The writer has overlooked the
fact that void* cannot (portably) be converted to or
from a function pointer type.

3) It speaks of malloc() returning its value "on the stack"
of the Alpha CPU, and conjures up the bogeyman of having
the wrong amount of stack space reserved. For functions
as simple as malloc(), Alpha didn't use the stack at all.
(Bad things would happen, but not the bad things that
the page describes.)

Finally, the danger that the page warns of ("You'll silence
the compiler's complaint about failing to include <stdlib.h>")
is largely irrelevant nowadays. All C99 compilers are obliged
to produce a diagnostic if an undeclared function is mentioned,
casts or no casts, many C90 compilers (not so obliged) do so out
of the goodness of their hearts, and every lint-ish tools I've
ever seen has done so. The error, if made, will be caught right
rapidly.

I happen to agree with the page's recommendation, but I
like to think my reasons are better than those it offers.

 

[lovecreatesbeauty]

I'm a long-time C programmer, and I don't agree that it should be removed.

The void * type was imitated from C++, which doesn't have the gratuitous hole
that void * can be converted to another pointer-to-object type without a cast:

double x;
void *pv = &x;
int *pi = pv; /* diagnostic required in C++, not in C. */

What it means is that you can write some unsafe conversions in C,
but there is no /required/ diagnosis for them. You may be able to find
such a diagnostic in your compiler anyway, in which case it's a good
idea to turn it on. Even if you don't have the diagnostic, it's still
a good idea to put in the cast.

h&s sec.6.4.1:
"in c++ a cast must be used to convert a pointer of type void* to
another kind of pointer. You can also use the cast in c, but it is not
required in an assignment"

h&s sec.16.1:
"the cast (T*) is not strictly necessary in standard c because malloc
returns a pointer of type void* and the implicit conversion on
assignment to objptr is allowed."

k&r errata #142:
"It's not necessary (given that coercion of void * to ALMOSTANYTYPE *
is automatic), and possibly harmful if malloc, or a proxy for it,
fails to be declared as returning void *. The explicit cast can cover
up an unintended error. On the other hand, pre-ANSI, the cast was
necessary, and it is in C++ also."

 

[Nick Keighley]

h&s sec.6.4.1:
"in c++ a cast must be used to convert a pointer of type void* to
another kind of pointer. You can also use the cast in c, but it is not
required in an assignment"

h&s sec.16.1:
"the cast (T*) is not strictly necessary in standard c because malloc
returns a pointer of type void* and the implicit conversion on
assignment to objptr is allowed."

k&r errata #142:
"It's not necessary (given that coercion of void * to ALMOSTANYTYPE *
is automatic), and possibly harmful if malloc, or a proxy for it,
fails to be declared as returning void *. The explicit cast can cover
up an unintended error. On the other hand, pre-ANSI, the cast was
necessary, and it is in C++ also."

so? Are you pointing out they disagree with Kaz?
He's probably aware of this

 

[Morris Keesan]

k&r errata #142:
On the other hand, pre-ANSI, the cast was
necessary [from malloc return to other pointer types]

I've seen this claim many times, but I've never used any compiler for
which the cast was necessary, nor seen any definition of the language
in which it was necessary, including, among others, the language
definition as published in K&R 1, or the (almost identical) C Language
documentation in the version 6 or 7 Unix documentation.