Memory Allocating problem

Discussion in 'C Programming' started by boss_bhat@yahoo.co.in, Jul 8, 2005.

  1. Guest

    Hi all ,
    I am beginner to C programming. I have a defined astructure like the
    following, and i am using aliases for the different data types in the
    structure,

    typedef struct _NAME_INFO {
    struct _NAME_INFO *Next;
    ULONG LastId;
    ULONG Id;
    PVOID Value;
    BOOLEAN Used;
    USHORT CodePage;
    WCHAR *Name;
    } NAME_INFO, *PNAME_INFO;

    When I allocate memory to the defined structure, in the following way,

    p = (PNAME_INFO)malloc(sizeof(PNAME_INFO)+ (wcslen(Name) + 1 ) *
    sizeof( WCHAR ));
    p->Name = (WCHAR *) malloc(( wcslen(Name) + 1 ) * sizeof( WCHAR ));
    It core dumps when allocating memory to the pointer to wide character
    *Name. Can somebody explain how to overcome this problem.

    Thanks in Advance

    Prasanna Bhat Mavinkuli
     
    , Jul 8, 2005
    #1
    1. Advertising

  2. Rajan Guest

    I think you should replace sizeof(PNAME_INFO) to sizeof(NAME_INFO).
     
    Rajan, Jul 8, 2005
    #2
    1. Advertising

  3. Guest

    wrote:
    > Hi all ,
    > I am beginner to C programming. I have a defined astructure like the
    > following, and i am using aliases for the different data types in the
    > structure,
    >
    > typedef struct _NAME_INFO {
    > struct _NAME_INFO *Next;
    > ULONG LastId;
    > ULONG Id;
    > PVOID Value;
    > BOOLEAN Used;
    > USHORT CodePage;
    > WCHAR *Name;
    > } NAME_INFO, *PNAME_INFO;
    >
    > When I allocate memory to the defined structure, in the following way,
    >
    > p = (PNAME_INFO)malloc(sizeof(PNAME_INFO)+ (wcslen(Name) + 1 ) *
    > sizeof( WCHAR ));


    1. remove the cast and include stdlib.h
    2. use p = malloc(sizeof(NAME_INFO));
    3. verify p is not NULL before using.

    > p->Name = (WCHAR *) malloc(( wcslen(Name) + 1 ) * sizeof( WCHAR ));
    > It core dumps when allocating memory to the pointer to wide character
    > *Name. Can somebody explain how to overcome this problem.
    >
    > Thanks in Advance
    >
    > Prasanna Bhat Mavinkuli
     
    , Jul 8, 2005
    #3
  4. CBFalconer Guest

    Rajan wrote:
    >
    > I think you should replace sizeof(PNAME_INFO) to sizeof(NAME_INFO).


    I think you should replace your newsreader with something that can
    create proper quotes, or should pay attention to my sig below.
    What you wrote is useless in isolation.

    --
    "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
     
    CBFalconer, Jul 8, 2005
    #4
  5. CBFalconer Guest

    wrote:
    >
    > I am beginner to C programming. I have a defined astructure like
    > the following, and i am using aliases for the different data types
    > in the structure,
    >
    > typedef struct _NAME_INFO {
    > struct _NAME_INFO *Next;
    > ULONG LastId;
    > ULONG Id;
    > PVOID Value;
    > BOOLEAN Used;
    > USHORT CodePage;
    > WCHAR *Name;
    > } NAME_INFO, *PNAME_INFO;
    >
    > When I allocate memory to the defined structure, in the following way,
    >
    > p = (PNAME_INFO)malloc(sizeof(PNAME_INFO)+ (wcslen(Name) + 1 ) * sizeof( WCHAR ));
    > p->Name = (WCHAR *) malloc(( wcslen(Name) + 1 ) * sizeof( WCHAR ));
    > It core dumps when allocating memory to the pointer to wide character
    > *Name. Can somebody explain how to overcome this problem.


    Try writing:

    if (NULL == (p = malloc(sizeof *p))) {
    /* take corrective action, no memory */;
    }
    else if (NULL ==
    (p->Name = malloc((sizeof *(p->Name)) * (1+wcslen(Name))) {
    /* take corrective action, no memory */
    }
    else {
    /* allocations successful, enjoy */
    }

    The only thing that has a need for a varying length is the Name
    field. The only thing the casts are doing for you is suppressing
    helpful compiler error messages. You didn't show the declaration
    for Name, so that may also be uninitialized.

    --
    "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
     
    CBFalconer, Jul 8, 2005
    #5
  6. writes:
    > I am beginner to C programming. I have a defined astructure like the
    > following, and i am using aliases for the different data types in the
    > structure,
    >
    > typedef struct _NAME_INFO {
    > struct _NAME_INFO *Next;
    > ULONG LastId;
    > ULONG Id;
    > PVOID Value;
    > BOOLEAN Used;
    > USHORT CodePage;
    > WCHAR *Name;
    > } NAME_INFO, *PNAME_INFO;


    Why are you using all these aliases? If PVOID is supposed to be a
    pointer to void, just use void*; if it isn't, the name PVOID is
    misleading.

    There's some debate as to whether typedefs for structs are a good
    idea. Some argue that just using "struct foo" directly is clearer
    than using a typedef name. A typedef hides the fact that the type is
    a struct. There are cases where you want to do this (e.g., the type
    FILE in <stdio.h>), but this isn't such a case.

    The name _NAME_INFO is potentially a problem. Many identifiers
    starting with an underscore are reserved to the implementation. (The
    rule is a bit more complicated, and has to do with what character
    follows the initial underscore, but it's best to avoid declaring
    things with leading underscores altogether.)

    If you're using a C99 compiler, you can use type "bool" if you have
    "#include <stdbool.h>". Otherwise, you can define type "bool"
    yourself. One approach is:

    #if __STDC_VERSION__ >= 199901L
    #include <stdbool.h>
    #else
    typedef int bool; /* or typedef char bool; */
    #define false 0
    #define true 1
    #endif

    So, given the above definition of "bool", and assuming I'm guessing
    correctly what your aliases mean, and assuming "#include <stddef.h>"
    for the definition of wchar_t, I'd declare your structure as follows:

    struct Name_Info {
    struct name_info *Next;
    unsigned long LastId;
    unsigned long Id;
    void *Value;
    bool Used;
    unsigned short CodePage;
    wchar_t *Name;
    };

    If you want to be able to refer to the type as "Name_Info" rather than
    "struct Name_Info", you can do this:

    typedef struct Name_Info {
    /* member declarations as above */
    } Name_Info;

    Structure tags and typedef names are in separate namespaces, so
    there's no need to use distinct identifiers. However, I probably
    wouldn't bother with the typedef; instead, I'd use "struct Name_Info"
    directly.

    I definitely wouldn't create a typedef for a pointer to a struct
    Name_Info. Hiding the fact that something is a pointer is likely to
    cause confusion.

    > When I allocate memory to the defined structure, in the following way,
    >
    > p = (PNAME_INFO)malloc(sizeof(PNAME_INFO)+ (wcslen(Name) + 1 ) *
    > sizeof( WCHAR ));
    > p->Name = (WCHAR *) malloc(( wcslen(Name) + 1 ) * sizeof( WCHAR ));
    > It core dumps when allocating memory to the pointer to wide character
    > *Name. Can somebody explain how to overcome this problem.


    If you're allocating an object of type "struct Name_Info", you just
    want to allocate the size of the struct. You could possibly allocate
    a "struct Name_Info" and the wide string to be pointed to by the Name
    member in a single chunk, but getting the alignment right would be
    difficult -- and since you're allocating space for p->Name separately,
    that's obviously not what you're trying to do anyway.

    Casting the result of malloc() is unnecessary and can mask the error
    of forgetting the "#include <stdlib.h>".

    So here's how I'd do the allocations:

    Name_Info *p;
    p = malloc(sizeof *p);
    if (p == NULL) {
    /* error handling */
    }
    p->Name = malloc((wcslen(Name) + 1) * sizeof wchar_t);
    if (p->Name == NULL) {
    /* error handling */
    }

    In the first malloc(), note that the argument is "sizeof *p". By
    referring to the pointer object and not to its type, we avoid problems
    if the type is changed later on. Since the expression "*p" is the
    operand of a sizeof, it's not evaluated; it's used only to determine
    the size of the type p points to -- which is exactly what we want.

    I'm assuming that the error handling code will bail out of whatever
    context you're in, so you won't attempt to assign to p->Name unless
    you know that the allocation for p has already succeeded.

    --
    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, Jul 8, 2005
    #6
  7. John Bode Guest

    wrote:
    > Hi all ,
    > I am beginner to C programming. I have a defined astructure like the
    > following, and i am using aliases for the different data types in the
    > structure,
    >


    Unless you have a really good reason for using typedef names for
    primitive data types (for example, you know USHORT will *always* be a
    particular size), use the regular type names, i.e., change

    > typedef struct _NAME_INFO {
    > struct _NAME_INFO *Next;
    > ULONG LastId;
    > ULONG Id;
    > PVOID Value;
    > BOOLEAN Used;
    > USHORT CodePage;
    > WCHAR *Name;
    > } NAME_INFO, *PNAME_INFO;
    >


    to

    typedef struct NAME_INFO { // don't use leading underscore
    struct NAME_INFO *Next;
    unsigned long LastId;
    unsigned long Id;
    void *Value; // I'm assuming PVOID == void*
    bool Used; // assuming bool type is available; otherwise int will
    work
    unsigned short CodePage;
    wchar_t *Name;
    } NAME_INFO;

    While I'll argue with Keith over the virtue of creating a typedef name
    for the struct type (I do it all the time, but I also create accessor
    functions to do member assignments), I won't argue about creating a
    typedef name for pointer types; don't do it. It will just cause
    heartburn. I know there are a lot of examples of the practice,
    especially in MS Windows code, but it winds up causing more problems
    than it allegedly solves.

    Just use NAME_INFO* when you need a pointer.

    > When I allocate memory to the defined structure, in the following way,
    >
    > p = (PNAME_INFO)malloc(sizeof(PNAME_INFO)+ (wcslen(Name) + 1 ) *
    > sizeof( WCHAR ));
    > p->Name = (WCHAR *) malloc(( wcslen(Name) + 1 ) * sizeof( WCHAR ));
    > It core dumps when allocating memory to the pointer to wide character
    > *Name. Can somebody explain how to overcome this problem.
    >


    You're allocating p incorrectly. Use the following (assuming p is type
    NAME_INFO *):

    p = malloc(sizeof *p);

    Note that I'm using sizeof on the actual object I'm allocating to, not
    its type. Also notice I'm not casting the result of malloc(). You
    don't need to cast, and doing so can mask a compile time error if you
    forget to #include <stdlib.h>.

    To allocate the Name member, do the following.

    if (p) // make sure p was successfully allocated
    {
    p->Name = malloc(sizeof *(p->Name) * wcslen(Name));
    }

    > Thanks in Advance
    >
    > Prasanna Bhat Mavinkuli
     
    John Bode, Jul 9, 2005
    #7
  8. "John Bode" <> writes:
    [snip]
    > Unless you have a really good reason for using typedef names for
    > primitive data types (for example, you know USHORT will *always* be a
    > particular size), use the regular type names, i.e., change


    To expand on that a bit, USHORT is obviously an abbreviation for
    "unsigned short". If it *is* unsigned short, it's much clearer to use
    unsigned short directly. If it *isn't* unsigned short, the name is
    grossly misleading.

    If you specifically want, say, a 16-bit unsigned type, a name like
    "uint16_t" is much clearer (and that's exactly the name used in C99's
    <stdint.h>). But be aware that not all implementations necessarily
    have a 16-bit unsigned type.

    --
    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, Jul 9, 2005
    #8
    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. hall
    Replies:
    4
    Views:
    459
  2. soni29
    Replies:
    6
    Views:
    447
    Kevin Goodsell
    Sep 5, 2003
  3. Axel
    Replies:
    1
    Views:
    769
    stephan beal
    Oct 27, 2003
  4. Sameer
    Replies:
    2
    Views:
    293
    David White
    Nov 3, 2003
  5. Rakesh Kumar
    Replies:
    5
    Views:
    695
    James Kanze
    Dec 21, 2007
Loading...

Share This Page