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

Discussion in 'C Programming' started by spasmous@yahoo.com, Aug 23, 2005.

  1. Guest

    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 :)
    , Aug 23, 2005
    #1
    1. Advertising

  2. <> wrote in message
    news:...

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

    return n;
    } /* initialize_f */

    -Charles
    Charles M. Reinke, Aug 23, 2005
    #2
    1. Advertising

  3. 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] = ...
    */

    }
    Kevin J. Phillips, Aug 23, 2005
    #3
  4. 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
    Emmanuel Delahaye, Aug 23, 2005
    #4
  5. Eric Sosman Guest

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

    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

    --
    Eric Sosman, Aug 23, 2005
    #5
  6. Re: malloc inside function (I know... I *did* search google first;)

    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.
    Martin Ambuhl, Aug 23, 2005
    #6
  7. Crap. Just noticed a mistake.

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


    Should be:
    *f = malloc(*n * sizeof **f);
    Kevin J. Phillips, Aug 23, 2005
    #7
  8. Re: malloc inside function (I know... I *did* search google first;)

    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) <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.
    Keith Thompson, Aug 23, 2005
    #8
  9. "Charles M. Reinke" <> wrote in message
    news:deg0ib$fcb$...
    > <> wrote in message
    > news:...
    >
    > 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 = 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
    Charles M. Reinke, Aug 23, 2005
    #9
  10. Michael Mair Guest

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

    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.

    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) = (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.
    Michael Mair, Aug 23, 2005
    #10
  11. Guest

    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 :)
    , Aug 23, 2005
    #11
  12. > 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.
    James Daughtry, Aug 23, 2005
    #12
  13. Roman Mashak Guest

    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:
    Roman Mashak, Aug 24, 2005
    #13
  14. Michael Mair Guest

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

    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:


    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.
    Michael Mair, Aug 24, 2005
    #14
  15. 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
    Lawrence Kirby, Aug 24, 2005
    #15
  16. Guest

    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 ;)
    , Aug 24, 2005
    #16
  17. Default User Guest

    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
    Default User, Aug 24, 2005
    #17
  18. Guest

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


    Default User wrote:
    > 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
    , Aug 24, 2005
    #18
  19. Default User Guest

    wrote:

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



    Sure, it identifies the idiots.

    *plonk*


    Brian
    Default User, Aug 24, 2005
    #19
  20. Re: malloc inside function (I know... I *did* search google first;)

    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) <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.
    Keith Thompson, Aug 24, 2005
    #20
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?TGFrc2htaSBOYXJheWFuYW4uUg==?=

    Google search result like site search!! How?

    =?Utf-8?B?TGFrc2htaSBOYXJheWFuYW4uUg==?=, May 5, 2005, in forum: ASP .Net
    Replies:
    3
    Views:
    659
    Lucas Tam
    May 6, 2005
  2. Replies:
    1
    Views:
    701
    cirfu
    Jun 19, 2008
  3. Daniel Waite
    Replies:
    2
    Views:
    220
    Daniel Waite
    May 2, 2008
  4. Andries

    I know, I know, I don't know

    Andries, Apr 23, 2004, in forum: Perl Misc
    Replies:
    3
    Views:
    222
    Gregory Toomey
    Apr 23, 2004
  5. Ray
    Replies:
    4
    Views:
    118
Loading...

Share This Page