malloc inside function (I know... I *did* search google first ;)

S

spasmous

main()
{
float * f;
initialize_f(f);

// ...use f for processing

free(f);
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

f[1] = ... // etc. fill f array here
}



The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)
 
C

Charles M. Reinke

You are missing a necessary header here:
#include <stdlib.h>
You're also missing a prototype for a funtion called in main(), try:
void initialize_f(float *f);

This should be:
int main(void)
Please refer to the c.l.c FAQ at http://www.faqs.org/faqs/C-faq/faq/ (which
you apparently DIDN'T find while googling), question #11.12a.
{
float * f;

Why use (float) instead of (double)?
initialize_f(f);

// ...use f for processing

free(f);

main() returns an int, you are missing something like:
return 0;
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

Casting the return value of malloc() is highly discouraged; see c.l.c FAQ
#7.7. In fact, in this case the absence of the unneccesary cast may have
given you a hint at compile time that you had forgotten to include a
required header file.
f[1] = ... // etc. fill f array here

You do realize, of course, that arrays in C start at index 0--what happens
to f[0] in your code?
}



The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)

I don't see any problems with the initialization of "f" in this code.
However your initialize_f() function should probably supply "n" as its
return value so that its size can be known in main(). I would write it
something like this:
/* Untested code follows */
long initialize_f(float *f) {
int i;
long n = 100;
f = malloc((size_t)n * sizeof *f);

for(i=0; i<n; i++) {
f = 0; /* or whatever initialization value you want */
} /* for i */

return n;
} /* initialize_f */

-Charles
 
K

Kevin J. Phillips

spasmous said:
main()
{
float * f;
initialize_f(f);

// ...use f for processing

free(f);
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

f[1] = ... // etc. fill f array here
}

The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)

No, that won't work because C is call by value. 'initialize_f'
gets a copy of 'f'. The copy is updated by the call to malloc()
but when the function returns, the value in the copy is lost.
You have to send initialize_f() a pointer to 'f'.
Try something like this instead.
Its untested and uncompiled, but it gets the idea across:

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

void initialize_f(float **f, long *n);

int main(void)
{
float *f;
long n;

initialize_f(&f, &n);
if (f == NULL)
exit(EXIT_FAILURE);

/* use f for processing ...
*
* if (f[0] < 3.14159) printf("almost pi!\n");
*/
...

/* All done, cleanup and quit */
free(f);

return 0;
}


void initialize_f(float **f, long *n)
{
*n = 100;
*f = malloc(n * sizeof **f);
if (*f == NULL)
{
fprintf(stderr, "malloc() failed\n");
return;
}

/* Initialize elements of f...
* (*f)[0] = 3.14159;
* (*f)[1] = ...
* ...
* (*f)[n-1] = ...
*/

}
 
E

Emmanuel Delahaye

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

You are modifying the value of a parameter. This often is the sign for
a bad design. Think more and you'll see why.

Note: parameters are passed by value in C.

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"It's specified. But anyone who writes code like that should be
transmogrified into earthworms and fed to ducks." -- Chris Dollin CLC
 
E

Eric Sosman

main()
{
float * f;
initialize_f(f);

// ...use f for processing

free(f);
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

f[1] = ... // etc. fill f array here
}



The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)

It will not. This is Question 4.8 in the comp.lang.c
Frequently Asked Questions (FAQ) list

http://www.eskimo.com/~scs/C-faq/top.html
 
M

Martin Ambuhl

(e-mail address removed) posted some error-intensive "code" which demonstrated
that he had not
1) followed the newsgroup before posting,
2) checked the FAQ before posting, or even
3) looked in a basic C text before posting:
main()
{
float * f;
initialize_f(f);

// ...use f for processing

free(f);
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

f[1] = ... // etc. fill f array here
}

I wrote a long post going over for what must be the 4000th time the
basic problems with the above. Only after I realized that nobody could
be both this stupid and this antisocial and be a legitimate poster that
I killed my response. This troll only got a nibble from me. He'll have
to try harder to get me to swallow the whole hook.
 
K

Keith Thompson

main()
{
float * f;
initialize_f(f);

// ...use f for processing

free(f);
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

f[1] = ... // etc. fill f array here
}



The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)

Calling malloc() inside a function doesn't itself cause any problems,
but this program won't work. The variable f in main() and the
parameter f in initialize_f() are two distinct objects. The parameter
gets the value of f when it's called, but that value isn't passed back
to main(). The parameter might as well be a local variable.

One approach is to pass a pointer to f:

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

void initialize_f(float **f);

int main(void)
{
float *f;
initialize_f(&f);

printf("f[1] = %g\n", f[1]);

free(f);
return 0;
}

void initialize_f(float **f_ptr)
{
long n = 100;
*f_ptr = malloc(n * sizeof **f_ptr);
if (*f_ptr == NULL) {
/* error handling */
}
else {
(*f_ptr)[1] = 123.456;
}
}


Some other changes: I declared initialize_f() before the call. I
added the required headers. In the malloc() call, I didn't
unnecessarily convert the result, and I modified the size expression
so it doesn't explicitly depend on the target type (so the malloc()
call doesn't have to change if the type is changed).

Perhaps a better approach is to return the value from the function:

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

float *initialize_f(void);

int main(void)
{
float *f;
f = initialize_f();

printf("f[1] = %g\n", f[1]);

free(f);
return 0;
}

float *initialize_f(void)
{
float *result;
long n = 100;
result = malloc(n*sizeof *result);
if (result == NULL) {
/* error handling */
}
else {
result[1] = 123.456;
}
return result;
}
 
C

Charles M. Reinke

Charles M. Reinke said:
You are missing a necessary header here:
#include <stdlib.h>
You're also missing a prototype for a funtion called in main(), try:
void initialize_f(float *f);

This should be:
int main(void)
Please refer to the c.l.c FAQ at http://www.faqs.org/faqs/C-faq/faq/ (which
you apparently DIDN'T find while googling), question #11.12a.
{
float * f;

Why use (float) instead of (double)?
initialize_f(f);

// ...use f for processing

free(f);

main() returns an int, you are missing something like:
return 0;
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

Casting the return value of malloc() is highly discouraged; see c.l.c FAQ
#7.7. In fact, in this case the absence of the unneccesary cast may have
given you a hint at compile time that you had forgotten to include a
required header file.
f[1] = ... // etc. fill f array here

You do realize, of course, that arrays in C start at index 0--what happens
to f[0] in your code?
}



The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)

I don't see any problems with the initialization of "f" in this code.
However your initialize_f() function should probably supply "n" as its
return value so that its size can be known in main(). I would write it
something like this:
/* Untested code follows */
long initialize_f(float *f) {
int i;
long n = 100;
f = malloc((size_t)n * sizeof *f);

for(i=0; i<n; i++) {
f = 0; /* or whatever initialization value you want */
} /* for i */

return n;
} /* initialize_f */

-Charles


Of course, I made (at least) 2 errors--f must be passed by address, i.e.
long initialize_f(float **f) {
/* corrected code*/
}
and the return value of malloc should ALWAYS be checked for NULL, i.e.
*f = malloc((size_t)n * sizeof **f);
if(*f==NULL) {
/* error handling code here */
}
else {
/* initialization code here */
}
....

-Charles
 
M

Michael Mair

Please post clean standard C code which compiles:

$ gcc -ansi -pedantic -Wall -O malloc.c
malloc.c:2: warning: return type defaults to `int'
malloc.c: In function `main':
malloc.c:4: warning: implicit declaration of function `initialize_f'
malloc.c:6: error: parse error before '/' token
malloc.c: At top level:
malloc.c:12: error: conflicting types for 'initialize_f'
malloc.c:4: error: previous implicit declaration of 'initialize_f' was here
malloc.c: In function `initialize_f':
malloc.c:14: warning: implicit declaration of function `malloc'
malloc.c:16: error: parse error before '...' token

It does not cost more than three extra lines to improve it.

Apart from the // comments being not part of C89 (which I inferred
from your use of main()), they also are considered dangerous in
usenet as they are not stable through a linebreak.


#include <stdlib.h>

void initialize_f (float *f);

int main (void)
{
float * f;
initialize_f(f);

// ...use f for processing
/* use f for processing */

return 0;
}

void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float));

f = malloc(n * sizeof *f);

is better C, does not mask the error of forgetting to
f[1] = ... // etc. fill f array here

You forgot a comment about safety checks on the return value
of malloc() or the actual checks.

f[0] = 0.0F;
/* etc fill f array here */
}

The size of the array is only known once initialize_f() is called so I
can't allocate it in main(). Please can someone tell me if f will be
initialized correctly by this code? Thanks :)

No.
If you want some object O of type T modified by a function,
you do not pass O but the address of O (i.e. a T* argument).
Otherwise, you only have a copy of O for which all steps
you deem necessary are performed -- but this copy of O dies
after the function is left and the original O is unchanged.

So, you want to pass &f to initialize_f, which consequently
has a float** parameter.

Finally, if you really work with array-like semantics for f,
it is better to pass the size info out of initialize_f(),
e.g. as a return value.

#include <stdlib.h>

size_t initialize_f (float **fptr);

int main (void)
{
float *f;
size_t fsize;

if (0 == (fsize = initialize_f(&f))) {
/* error handling */
}

/* use f */

free(f);

return 0;
}

size_t initialize_f (float **fptr)
{
size_t i, n = 100;
*fptr = malloc(n * sizeof **fptr);

if (NULL == *fptr) {
/* maybe error handling; cheap alternative: */
return 0;
}

for (i=0; i<n; i++)
(*fptr) = (float)i/n;

return n;
}

Note: float is almost never the right type for floating
point values -- you run relatively early into precision
and rounding error issues at (most of the time) no gain.
Use double by default and float where you need it.


Cheers
Michael
 
S

spasmous

Wow, you boys are hardcore on here! Rock on.

So I think I understand the problem better now, f needs to be passed as
a pointer to a pointer. Thanks :)
 
J

James Daughtry

main()
This is a confusing definition for main, and it encourages undefined
behavior by making newcomers think that main returns nothing when in
fact it returns int. Be explicit because eventually the C99 standard
will become widely implemented and this definition will fail to
compile.

int main(void)
{
/* Your code here */
return 0;
}
void initialize_f(float * f)
{
long n = 100;
f = (float*)malloc(n*sizeof(float)­);

f[1] = ... // etc. fill f array here
}

This is all kinds of wrong. Because f is a copy of the original
pointer, you lose the memory you assign to it when the function
returns. You also cast malloc, thus hiding the error of not including
stdlib.h. Also, since the size of the array is a local variable, and
initialize_f returns void, how does the calling function know how much
memory was allocated? You also never declare initialize_f before using
it in main. How about this instead:

#include <stdlib.h>

int initialize_f(float **f)
{
int n = 100;

*f = malloc(n * sizeof(float));

/* Fill the array */

return n;
}

int main(void)
{
float *f;
int n = initialize_f(&f);

/* Use f for processing */

free(f);

return 0;
}

Of course, it looks to me like you could more easily keep the
allocation and release in main by having a function that returns n,
allocating the memory in main using that n, then calling initialize_f
to fill it with whatever values you're using. It's easier to keep your
head about you if you avoid allocating memory in one function and
releasing it elsewhere.
 
R

Roman Mashak

Hello, Michael!
You wrote on Tue, 23 Aug 2005 22:47:33 +0200:

[skip]
MM> return 0;
MM> }

MM> size_t initialize_f (float **fptr)
MM> {
MM> size_t i, n = 100;
Tiny question: what's the reason of declaring function's return type as
'size_t' ?
MM> *fptr = malloc(n * sizeof **fptr);

With best regards, Roman Mashak. E-mail: (e-mail address removed)
 
M

Michael Mair

Roman said:
Hello, Michael!
You wrote on Tue, 23 Aug 2005 22:47:33 +0200:

[skip]
MM> return 0;
MM> }

MM> size_t initialize_f (float **fptr)
MM> {
MM> size_t i, n = 100;
Tiny question: what's the reason of declaring function's return type as
'size_t' ?
MM> *fptr = malloc(n * sizeof **fptr);

With best regards, Roman Mashak. E-mail: (e-mail address removed)

You snipped the answer given by me: If f is used to access an "array"
instead of, say the head of a linked list, it is often advantageous
to pass along the information how large the storage pointed to by f
is. There are other situations where you want to hide exactly this
information from the user but they are comparatively rare. A return
value is often a better solution to overcome the difficulty of
communicating the size than using file scope identifiers with static
or external linkage.
If the information is not needed right now: fine. Then just use the
return value to detect success/failure in the calling functions.
I just hate the concept of working with an object used as array
without knowing the size -- then often implicit assumptions are
used like "n is known to be >= 3". This is crap as the base for
the assumption may subtly change or someone made a simple mistake.

Personally, I use void return value mostly for functions which
cannot fail and have/gather no information the caller does not
have at its fingertips but which he is allowed to have.


Cheers
Michael
 
L

Lawrence Kirby

On Tue, 23 Aug 2005 16:36:59 -0400, Charles M. Reinke wrote:

....
/* Untested code follows */
long initialize_f(float *f) {
int i;
long n = 100;
f = malloc((size_t)n * sizeof *f);

for(i=0; i<n; i++) {
f = 0; /* or whatever initialization value you want */
} /* for i */

return n;
} /* initialize_f */

-Charles


Of course, I made (at least) 2 errors--f must be passed by address, i.e.
long initialize_f(float **f) {
/* corrected code*/
}
and the return value of malloc should ALWAYS be checked for NULL, i.e.
*f = malloc((size_t)n * sizeof **f);
if(*f==NULL) {
/* error handling code here */
}
else {
/* initialization code here */
}
...


Or IMO better still make the pointer the return value of the function

float *initialize_f(long *pn)
{
long i;
long n = 100;
float *f = malloc((size_t)n * sizeof *f);

if (f != NULL) {
for (i=0; i<n; i++) {
f = 0; /* or whatever initialization value you want */
}

if (pn != NULL)
*pn = n;
}

return f;
}

Functions that allocate memory commonly return a pointer to the memory
allocated. That pointer is the primary return value from the function and
a null return value can be used to indicate failure as other functions
like malloc() do. The size value may or may not be needed depending on
context. The code allows a null pointer to be passed if it is not needed.
If it is never needed the pn parameter can be omitted completely from the
function definition, and then the float * return is certainly more natural
than using a float ** parameter and no return value.

Also some thought should be put into the function name. "initialize_f" is
a misleading name for a function that allocates the array, not just
initialises it.

Lawrence
 
S

spasmous

Thanks Lawrence, at last some good advice here! I much prefer that way.
It's funny, the cranks here got so worked up over coding style they
forgot about substance ;)
 
D

Default User

Thanks Lawrence, at last some good advice here! I much prefer that
way. It's funny, the cranks here got so worked up over coding style
they forgot about substance ;)

See any of a dozen recent messages about quoting some of the previous
message and how to do it from Google.



Brian
 
S

spasmous

Do you like top posting as well? I do :)


Default said:
See any of a dozen recent messages about quoting some of the previous
message and how to do it from Google.



Brian
 
K

Keith Thompson

Thanks Lawrence, at last some good advice here! I much prefer that way.
It's funny, the cranks here got so worked up over coding style they
forgot about substance ;)

First, please provide context (and in reference to your later
followup, please don't top-post).

Second, there's no reason we can't both answer the substance of your
question and offer suggestions about coding style. I suggested using
a function in a followup I posted yesterday.

Note, however, that if the function is expected both to allocate and
initialize the array *and* to determine its size, you'll want to have
it return both the allocated size and a pointer to the array to the
caller. That's more difficult to do through a function result. You
could return a struct, but that might not fit as well into the context
in which the function is called.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top