469,621 Members | 1,678 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 469,621 developers. It's quick & easy.

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

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 :)

Nov 15 '05 #1
20 10399
<sp******@yahoo.com> wrote in message
news:11**********************@g43g2000cwa.googlegr oups.com...

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);
main()
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[i] = 0; /* or whatever initialization value you want */
} /* for i */

return n;
} /* initialize_f */

-Charles
Nov 15 '05 #2
spasmous wrote:
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] = ...
*/

}
Nov 15 '05 #3
sp******@yahoo.com wrote on 23/08/05 :
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
Nov 15 '05 #4


sp******@yahoo.com wrote:
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

--
Er*********@sun.com

Nov 15 '05 #5
sp******@yahoo.com 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.
Nov 15 '05 #6

Crap. Just noticed a mistake.

I wrote:
*f = malloc(n * sizeof **f);


Should be:
*f = malloc(*n * sizeof **f);

Nov 15 '05 #7
sp******@yahoo.com writes:
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;
}

--
Keith Thompson (The_Other_Keith) ks***@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.
Nov 15 '05 #8
"Charles M. Reinke" <cm******@ece.gatech.edu> wrote in message
news:de**********@news-int.gatech.edu...
<sp******@yahoo.com> wrote in message
news:11**********************@g43g2000cwa.googlegr oups.com...

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);
main()
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[i] = 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
Nov 15 '05 #9
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.

sp******@yahoo.com wrote:
main()
#include <stdlib.h>

void initialize_f (float *f);

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

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

free(f);
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
#include <stdlib.h> and is stable through a type change
of f.

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)[i] = (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
--
E-Mail: Mine is an /at/ gmx /dot/ de address.
Nov 15 '05 #10
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 :)

Nov 15 '05 #11
> 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.

Nov 15 '05 #12
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: mr*@tusur.ru
Nov 15 '05 #13
Roman Mashak wrote:
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: mr*@tusur.ru


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
--
E-Mail: Mine is an /at/ gmx /dot/ de address.
Nov 15 '05 #14
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[i] = 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[i] = 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

Nov 15 '05 #15
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 ;)

Nov 15 '05 #16
sp******@yahoo.com wrote:
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
Nov 15 '05 #17
Do you like top posting as well? I do :)
Default User wrote:
sp******@yahoo.com wrote:
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


Nov 15 '05 #18
sp******@yahoo.com wrote:
Do you like top posting as well? I do :)

Sure, it identifies the idiots.

*plonk*
Brian

Nov 15 '05 #19
sp******@yahoo.com writes:
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.

--
Keith Thompson (The_Other_Keith) ks***@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.
Nov 15 '05 #20
sp******@yahoo.com wrote: *** and top-posted - fixed ***
Default User wrote:
sp******@yahoo.com wrote:
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.


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


Do not top-post in any technical newsgroup. Your answer belongs
after, or possibly intermixed with, the material to which you
reply, after snipping out portions not germane to your answer. Top
posting is considered rude and a sign of ignorance.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
Nov 15 '05 #21

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

116 posts views Thread by Kevin Torr | last post: by
35 posts views Thread by ytrama | last post: by
25 posts views Thread by Why Tea | last post: by
58 posts views Thread by Jorge Peixoto de Morais Neto | last post: by
71 posts views Thread by desktop | last post: by
reply views Thread by devrayhaan | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.