why is casting malloc a bad thing?

Discussion in 'C Programming' started by Brian Blais, Jan 23, 2004.

  1. Brian Blais

    Brian Blais Guest

    Hello,

    I saw on a couple of recent posts people saying that casting the return
    value of malloc is bad, like:

    d=(double *) malloc(50*sizeof(double));

    why is this bad? I had always thought (perhaps mistakenly) that the
    purpose of a void pointer was to cast into a legitimate date type. Is
    this wrong? Why, and what is considered to be correct form?

    thanks,

    Brian Blais
    Brian Blais, Jan 23, 2004
    #1
    1. Advertising

  2. Brian Blais wrote:
    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:
    >
    > d=(double *) malloc(50*sizeof(double));
    >
    > why is this bad? I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type. Is
    > this wrong? Why, and what is considered to be correct form?


    AFAIK it is wrong as <stdlib.h> should already have declared malloc the right
    way. By overriding the standard declaration, you risk to re-declare malloc() in
    a way it is not ment to. In other words, the <stdlib.h> declaration is the only
    correct one, it must work and if it does not it means either that it's broken
    or that <stdlib.h> has not been included.

    A nice description of the "problem" can be found in:
    http://users.powernet.co.uk/eton/c/hackerhotel.html

    Ciao,
    --
    Roberto Divia` Love at first sight is one of the greatest
    ============= labour-saving devices the world has ever seen.
    Mailbox: C02110 CERN-European Organization for Nuclear Research
    E-mail: CH-1211 GENEVE 23, Switzerland
    Roberto DIVIA, Jan 23, 2004
    #2
    1. Advertising

  3. Brian Blais <> scribbled the following:
    > Hello,


    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:


    > d=(double *) malloc(50*sizeof(double));


    > why is this bad? I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type. Is
    > this wrong? Why, and what is considered to be correct form?


    It's not exactly _wrong_ but doesn't help anything either. Why people
    discourage it is because of these reasons:
    1) There is no need to. malloc(), correctly prototyped, returns a
    void *, which is assignable to any object pointer type anyway.
    2) If malloc() is not correctly prototyped, casting the return value
    won't fix anything. malloc() will still return int, and casting that
    int to an object pointer type isn't magically going to change it to a
    pointer.
    3) If malloc() is not correctly prototyped, the code is broken, because
    it causes undefined behaviour. However casting the malloc() will make
    the compiler _think_ it's not broken, although it really is.
    The correct form is simply:
    d = malloc(50*sizeof(double));
    or:
    d = malloc(50*sizeof *d);

    --
    /-- Joona Palaste () ------------- Finland --------\
    \-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
    "Hasta la Vista, Abie!"
    - Bart Simpson
    Joona I Palaste, Jan 23, 2004
    #3
  4. Brian Blais <> wrote in news::

    > Hello,
    >
    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:
    >
    > d=(double *) malloc(50*sizeof(double));


    See the C-FAQ as to why.

    http://www.eskimo.com/~scs/C-faq/q7.6.html
    http://www.eskimo.com/~scs/C-faq/q7.7.html

    > why is this bad? I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type.


    Negative. Void pointers can hold any type of pointer so they naturally
    "become" the desired type.

    E.g.

    void foo(void *pThing)
    {
    int idx;
    char *pChar = pThing;

    /* work with pChar */
    for (idx = 0; idx < 10; ++idx)
    {
    pChar[idx] = 'a';
    }
    }

    void bar(void)
    {
    int arr[100];

    foo(arr);
    }

    No icky casts needed or desired. Void is your friend.

    --
    - Mark ->
    --
    Mark A. Odell, Jan 23, 2004
    #4
  5. Brian Blais

    Artie Gold Guest

    Brian Blais wrote:
    > Hello,
    >
    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:
    >
    > d=(double *) malloc(50*sizeof(double));
    >
    > why is this bad? I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type. Is
    > this wrong? Why, and what is considered to be correct form?
    >


    A pointer to void (void *) can be *implicitly* converted from and to a
    pointer to any object type, making the cast unnecessary. It is likely
    that the confusion arises from the fact that this was not the case in
    pre-ANSI (referred to as K&R) C, where the explicit cast *was* necessary.

    The preferred for of the call to malloc() above is:

    d = malloc(50 * sizeof *d);

    This has several advantages:

    It's shorter. ;-)

    If one fails to #include <stdlib.h> (in which case the return type of
    malloc() reverts to implicit `int') an error will be generated. This is
    particularly significant on platforms where the size of `int' and `void
    *' differ; in such cases the explicit cast would allow the code to
    compile, but behave strangely..

    If the type of `d' changes, the form of the malloc() call does not have
    to be changed.

    In general, casting is evil, except where it's necessary. Since it isn't
    necessary in this case, why do it?


    This issue has appeared repeatedly on news:comp.lang.c. If you have
    further questions, I would recommend searching the archives (through,
    for example, http://groups.google.com).

    HTH,
    --ag
    --
    Artie Gold -- Austin, Texas
    Artie Gold, Jan 23, 2004
    #5
  6. Brian Blais

    Richard Bos Guest

    Brian Blais <> wrote:

    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:
    >
    > d=(double *) malloc(50*sizeof(double));
    >
    > why is this bad?


    For the same reason that hanging a "danger - high voltage" sign on a
    normal battery is wrong.
    Casts are used to circumvent the C type system. If you use them where
    the type system does not need to be circumvented, you give off the wrong
    signals: that something odd is going on on that line (which it isn't),
    and that you're unsure about how the type system works (which you
    shouldn't be).

    > I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type. Is
    > this wrong?


    This is quite the opposite of right. The purpose of a void * is to help
    you _not_ need useless casts all over the place.

    Richard
    Richard Bos, Jan 23, 2004
    #6
  7. Brian Blais

    xarax Guest

    "Brian Blais" <> wrote in message
    news:...
    > Hello,
    >
    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:
    >
    > d=(double *) malloc(50*sizeof(double));
    >
    > why is this bad? I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type. Is
    > this wrong? Why, and what is considered to be correct form?
    >
    > thanks,


    You will find folks on both sides of the issue.
    The main (some would say *only*) argument against
    casting the result is that it will hide the failure
    to include <stdlib.h> which declares malloc to return
    (void *). The compiler will assume it returns (int),
    and casting (int) to (something *) will cause undefined
    behavior.

    The folks on the other side of the argument say that
    their compiler will complain loudly when a function
    is referenced without a declaration (implementation-defined behavior), so the
    question about including <stdlib.h>
    is moot. Once past the question of proper malloc()
    declaration, one must contend with the whether the
    target pointer is what one intended.

    ======================
    #include <stdlib.h>

    static void fubar(void)
    {
    float * d; /* programmer changed this...! */

    /* some dozen lines later in the code */

    /* silent erroneous size calculation */
    d = malloc(50*sizeof(double));
    }
    ======================

    If the programmer decides to change the declaration
    of "d", the compiler will *NOT* catch the bad assignment.

    If the assignment used a cast (double *), then the
    compiler would complain about incompatible pointer
    types.

    ======================
    #include <stdlib.h>

    static void fubar(void)
    {
    float * d; /* programmer changed this...! */

    /* some dozen lines later in the code */

    /* incompatible pointer assignment */
    d = (double *) malloc(50*sizeof(double));
    }
    ======================



    Another suggestion is *not* to use "sizeof(double)",
    but use "sizeof *d", so that the element size changes
    automatically when changing the target declaration.

    ======================
    #include <stdlib.h>

    static void fubar(void)
    {
    struct gonk * d; /* programmer changed this...! */

    /* some dozen lines later in the code */

    /* 50 elements of whatever "d" points to... */
    d = malloc(50*sizeof *d);
    }
    ======================

    Now the malloc() is compatible with changing the
    target type of the pointer variable "d". You can
    change it to any kind of pointer type and the
    malloc will be compatible. This is the recommended
    way to use malloc(), so that:

    1. A missing <stdlib.h> is caught.

    2. The malloc() assignment is always compatible
    with any pointer type.

    3. The size calculation changes along with any
    change to the target declaration.


    If you insist on using sizeof(double) in the size
    calculation, then you should cast the result so that
    the compiler will complain when the target type is
    changed (so that you can change the sizeof() and
    recompile). However, you will save yourself some
    maintenance headaches by using: "d = malloc(50*sizeof *d);"



    2 cents worth. Your mileage may vary.

    --
    ----------------------------
    Jeffrey D. Smith
    Farsight Systems Corporation
    24 BURLINGTON DRIVE
    LONGMONT, CO 80501-6906
    http://www.farsight-systems.com
    z/Debug debugs your Systems/C programs running on IBM z/OS!
    Are ISV upgrade fees too high? Check our custom product development!
    xarax, Jan 23, 2004
    #7
  8. xarax <> scribbled the following:
    > "Brian Blais" <> wrote in message
    > news:...
    >> Hello,
    >>
    >> I saw on a couple of recent posts people saying that casting the return
    >> value of malloc is bad, like:
    >>
    >> d=(double *) malloc(50*sizeof(double));
    >>
    >> why is this bad? I had always thought (perhaps mistakenly) that the
    >> purpose of a void pointer was to cast into a legitimate date type. Is
    >> this wrong? Why, and what is considered to be correct form?
    >>
    >> thanks,


    > You will find folks on both sides of the issue.
    > The main (some would say *only*) argument against
    > casting the result is that it will hide the failure
    > to include <stdlib.h> which declares malloc to return
    > (void *). The compiler will assume it returns (int),
    > and casting (int) to (something *) will cause undefined
    > behavior.


    > The folks on the other side of the argument say that
    > their compiler will complain loudly when a function
    > is referenced without a declaration (implementation-defined behavior), so the
    > question about including <stdlib.h>
    > is moot. Once past the question of proper malloc()
    > declaration, one must contend with the whether the
    > target pointer is what one intended.


    I don't understand this argument at all. Without the correct
    prototype, using the return value of malloc() causes undefined
    behaviour, *CAST OR NO CAST*. The code is broken anyway. *With*
    the correct prototype, the cast is completely needless.

    > ======================
    > #include <stdlib.h>


    > static void fubar(void)
    > {
    > float * d; /* programmer changed this...! */


    > /* some dozen lines later in the code */


    > /* silent erroneous size calculation */
    > d = malloc(50*sizeof(double));
    > }
    > ======================


    > If the programmer decides to change the declaration
    > of "d", the compiler will *NOT* catch the bad assignment.


    This is why most folks prefer to write:
    d = malloc(50*sizeof *d);
    and let that solve the problem for them.

    > If the assignment used a cast (double *), then the
    > compiler would complain about incompatible pointer
    > types.


    Which would not have ever arisen if they had used the form I
    presented above.

    > ======================
    > #include <stdlib.h>


    > static void fubar(void)
    > {
    > float * d; /* programmer changed this...! */


    > /* some dozen lines later in the code */


    > /* incompatible pointer assignment */
    > d = (double *) malloc(50*sizeof(double));
    > }
    > ======================


    > Another suggestion is *not* to use "sizeof(double)",
    > but use "sizeof *d", so that the element size changes
    > automatically when changing the target declaration.


    Yes, like I just described.

    (Snip example)

    --
    /-- Joona Palaste () ------------- Finland --------\
    \-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
    "'It can be easily shown that' means 'I saw a proof of this once (which I didn't
    understand) which I can no longer remember'."
    - A maths teacher
    Joona I Palaste, Jan 23, 2004
    #8
  9. Brian Blais

    Mark Bruno Guest

    Casting of void* is required in C++, so i usually do it in C also. It's not
    required, but it makes it easier when switching between C and C++, like I
    often do.
    Mark Bruno, Jan 23, 2004
    #9
  10. "Mark Bruno" <> wrote in
    news::

    > Casting of void* is required in C++, so i usually do it in C also. It's
    > not required, but it makes it easier when switching between C and C++,
    > like I often do.


    So are you saying if I didn't name all my loop variables 'new', 'delete',
    'cout', 'cin', etc. I'd have an easier time porting? BTW, why would you
    ever use malloc() in C++?

    --
    - Mark ->
    --
    Mark A. Odell, Jan 23, 2004
    #10
  11. Brian Blais

    Default User Guest

    Mark Bruno wrote:
    >
    > Casting of void* is required in C++, so i usually do it in C also. It's not
    > required, but it makes it easier when switching between C and C++, like I
    > often do.



    You shouldn't be using malloc() in C++ period. Writing code to move back
    and forth like that is poor idea. C++ is its own language, and you
    should be writing C++ code using the modern libraries. Don't write
    cripple C code for C++ applications.

    I work in both languages, it's not that hard.



    Brian Rodenborn
    Default User, Jan 23, 2004
    #11
  12. Brian Blais

    j Guest

    "Brian Blais" <> wrote in message
    news:...
    > Hello,
    >
    > I saw on a couple of recent posts people saying that casting the return
    > value of malloc is bad, like:
    >
    > d=(double *) malloc(50*sizeof(double));
    >
    > why is this bad? I had always thought (perhaps mistakenly) that the
    > purpose of a void pointer was to cast into a legitimate date type. Is
    > this wrong? Why, and what is considered to be correct form?
    >
    > thanks,
    >


    You do not have to cast a pointer to void as it can be converted to or from
    a pointer to any incomplete or object type.

    The issue with casting from malloc is not that it is just superfluous
    but that you might be suppressing a warning that might otherwise
    be useful.


    > Brian Blais
    >
    j, Jan 23, 2004
    #12
  13. Brian Blais wrote:

    > I saw on a couple of recent posts
    > people saying that casting the return value of malloc is bad, like:
    >
    > double* p = (double*)malloc(50*sizeof(double));
    >
    > Why is this bad?


    It isn't.

    > I had always thought (perhaps mistakenly) that
    > the purpose of a void pointer was to cast into a legitimate date type.
    > Is this wrong?


    No.

    > Why and what is considered to be correct form?


    This is just a style issue.

    I always use the explicit cast
    to prevent my C++ compiler from complaining about

    warning: invalid conversion from `void*' to `double*'

    It applies the implicit conversion anyway so it doesn't matter
    except that more important warnings some times get lost
    if too many warning messages such as this are issued by my compiler.

    The important thing here is to adopt a style and stick to it.
    E. Robert Tisdale, Jan 23, 2004
    #13
  14. "E. Robert Tisdale" <> wrote in
    news::

    Do you really work for NASA yet spew such rubbish?

    >> I saw on a couple of recent posts
    >> people saying that casting the return value of malloc is bad, like:
    >>
    >> double* p = (double*)malloc(50*sizeof(double));
    >>
    >> Why is this bad?

    >
    > It isn't.


    It is not proper C. To the OP, ignore E. Robert Tisdale's response and
    read the C-FAQ instead.

    >> I had always thought (perhaps mistakenly) that
    >> the purpose of a void pointer was to cast into a legitimate date type.
    >> Is this wrong?

    >
    > No.


    It is.

    >> Why and what is considered to be correct form?

    >
    > This is just a style issue.


    It is not.

    > I always use the explicit cast
    > to prevent my C++ compiler from complaining about


    You *shouldn't* use malloc in C++!

    > The important thing here is to adopt a style and stick to it.


    True but this is not a style issue.

    --
    - Mark ->
    --
    Mark A. Odell, Jan 23, 2004
    #14
  15. Mark A. Odell wrote:

    > It is not proper C.


    Nonsense!
    An explicit cast on the value returned from malloc
    is *not* explicitly os implicitly prohibited by the ANSI/ISO C standards
    and there is *no* C compiler that will complain about it.

    Who said anything about using malloc in C++?
    I'm just using my C++ compiler to compile C code.

    >>The important thing here is to adopt a style and stick to it.

    >
    > True but this is not a style issue.


    If it isn't a style issue, then it is nothing at all.
    E. Robert Tisdale, Jan 23, 2004
    #15
  16. "E. Robert Tisdale" <> wrote in
    news::

    > Mark A. Odell wrote:
    >
    >> It is not proper C.

    >
    > Nonsense!
    > An explicit cast on the value returned from malloc
    > is *not* explicitly os implicitly prohibited by the ANSI/ISO C standards
    > and there is *no* C compiler that will complain about it.


    It's not illegal, just not proper. Casts are a punt, you are side-stepping
    the C type enforcement. In some cases you must do this (cast) but malloc()
    is definitely not one of them.

    >
    > Who said anything about using malloc in C++?
    > I'm just using my C++ compiler to compile C code.


    Why when you can tell it to switch ISO C mode? Usually, just giving the
    file a .c extension is all that is required. Otherwise, a flag is usually
    available to force the file to be treated as C not C++.

    >>>The important thing here is to adopt a style and stick to it.

    >>
    >> True but this is not a style issue.

    >
    > If it isn't a style issue, then it is nothing at all.


    Oh, it's still a punt.

    --
    - Mark ->
    --
    Mark A. Odell, Jan 23, 2004
    #16
  17. Brian Blais

    Eric Sosman Guest

    "E. Robert Tisdale" wrote:
    >
    > Mark A. Odell wrote:
    >
    > > It is not proper C.

    >
    > Nonsense!
    > An explicit cast on the value returned from malloc
    > is *not* explicitly os implicitly prohibited by the ANSI/ISO C standards
    > and there is *no* C compiler that will complain about it.


    "Not proper" is perhaps too strong. "Not useful" or
    "not needed" or "not smart" might have been better.

    > Who said anything about using malloc in C++?
    > I'm just using my C++ compiler to compile C code.


    It's not C code if it's being compiled as C++. Perhaps
    you could clarify with a simple test:

    #include <stdio.h>
    typedef int private;
    private main(private class, char **new) {
    puts ("Hello, world!");
    return 0;
    }

    If your implementation fails to compile and run this program,
    the code is not being processed as C but as something else.

    --
    Eric Sosman, Jan 23, 2004
    #17
  18. Eric Sosman wrote:

    > Perhaps you could clarify with a simple test:
    >
    > #include <stdio.h>
    > typedef int private;
    > private main(private class, char **new) {
    > puts ("Hello, world!");
    > return 0;
    > }
    >
    > If your implementation fails to compile and run this program,
    > the code is not being processed as C but as something else.


    > cat main.c

    #include <stdio.h>

    typedef int private;
    private main(private class, char* new[]) {
    puts("Hello, world!");
    return 0;
    }

    > g++ -x c++ -Dprivate=Private -Dclass=Class -Dnew=New \

    -Wall -ansi -pedantic -o main main.c
    main.c:4: warning: return type for `main' changed to `int'
    > ./main

    Hello, world!
    > gcc -x c -Dprivate=Private -Dclass=Class -Dnew=New \

    -Wall -std=c99 -pedantic -o main main.c
    > ./main

    Hello, world!
    E. Robert Tisdale, Jan 23, 2004
    #18
  19. Brian Blais

    Eric Sosman Guest

    "E. Robert Tisdale" wrote:
    >
    > Eric Sosman wrote:
    >
    > > Perhaps you could clarify with a simple test:
    > >
    > > #include <stdio.h>
    > > typedef int private;
    > > private main(private class, char **new) {
    > > puts ("Hello, world!");
    > > return 0;
    > > }
    > >
    > > If your implementation fails to compile and run this program,
    > > the code is not being processed as C but as something else.

    >
    > > cat main.c

    > #include <stdio.h>
    >
    > typedef int private;
    > private main(private class, char* new[]) {
    > puts("Hello, world!");
    > return 0;
    > }
    >
    > > g++ -x c++ -Dprivate=Private -Dclass=Class -Dnew=New \

    > -Wall -ansi -pedantic -o main main.c
    > main.c:4: warning: return type for `main' changed to `int'
    > > ./main

    > Hello, world!
    > > gcc -x c -Dprivate=Private -Dclass=Class -Dnew=New \

    > -Wall -std=c99 -pedantic -o main main.c
    > > ./main

    > Hello, world!


    Care to try it again without the -D switches?

    Here's the point I'm trying to make: The meaning of a source
    file is not contained within the file itself, but only arises
    when the file is handed to a compiler or interpreter or whatever.
    A C compiler treats the source handed to it as C code; hand that
    same source to a C++ compiler and it's C++ code. Hand it to a
    COBOL compiler and it's COBOL code.

    I once saw an ingeniously constructed source file that
    implemented "Hello, world!" in three languages at once: C (pre-
    Standard), Fortran (flavor unremembered), and csh. And in natural
    languages we have the example of "Mots D'Heures, Gousses, Rames,"
    a text that is simultaneously stilted French and phonetic English.
    Such exercises, although entertaining and perhaps awe-inspiring,
    are not especially practical.

    Ordinarily, it's difficult to prepare a source file that's
    meaningful in two or more different languages, and even more
    difficult to get the different languages to assign the source the
    same meaning. But because C and C++ diverged fairly recently,
    they have enough in common that it's still possible to write such
    ambivalent code fragments. "Possible" doesn't mean "Recommended"
    or even "Sane," though: The writing is noticeably cramped, even
    if not as perversely contorted as the ForcshtranC tour de farce.
    Some may find it amusing to write such hermaphroditic code, but
    a recommendation for or against a particular coding practice
    should arise from something more than entertainment potential.

    Do not pretend that C and C++ are interchangeable, even in
    subsetted forms. They are closely related but different languages,
    and the attempt to write ambivalent code gives a product that is
    neither good C nor good C++. Choose your language and use it to
    the hilt; there is no reason to weaken your code with compromise.

    --
    Eric Sosman, Jan 23, 2004
    #19
  20. Eric Sosman wrote:

    > Here's the point I'm trying to make:


    [snip]

    The point is that nobody claimed that,
    "Every C program is a C++ program".

    I can't afford to write C code
    that cannot also be compiled by a C++ compiler.
    We can't afford to support two versions of libraries
    one for C programs and another for C++ programs.
    We just write code that will compile as either one.
    E. Robert Tisdale, Jan 24, 2004
    #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. Mr. SweatyFinger
    Replies:
    2
    Views:
    1,763
    Smokey Grindel
    Dec 2, 2006
  2. Ronny Mandal

    Strange thing about malloc()

    Ronny Mandal, Apr 19, 2005, in forum: C Programming
    Replies:
    16
    Views:
    557
    Alan Balmer
    Apr 22, 2005
  3. Barry
    Replies:
    39
    Views:
    2,092
    Jerry Coffin
    Aug 7, 2007
  4. rantingrick
    Replies:
    44
    Views:
    1,177
    Peter Pearson
    Jul 13, 2010
  5. Zam
    Replies:
    1
    Views:
    222
    Mark Schupp
    Mar 14, 2005
Loading...

Share This Page