Quick question on malloc and strings

This is a discussion on Quick question on malloc and strings within the C Programming forums, part of the General Programming Boards category; I'm trying to get the hang of using pointers for strings instead of arrays. I'm still finding it a little ...

  1. #1
    Registered User officedog's Avatar
    Join Date
    Oct 2008
    Posts
    77

    Quick question on malloc and strings

    I'm trying to get the hang of using pointers for strings instead of arrays. I'm still finding it a little confusing. Just wondering if I could get some feedback - does this look OK? It works so far, but ....

    Code:
    int length;
    char *word;
    
    void addWord(const char *newWord){
        length = strlen(newWord) +1;
        
        word = NULL;
        word = (char *) malloc(length);
        if (word != NULL)
        {
            strlcpy(word, newWord, length); //strlcpy adds '\0'
        }
        else
        {
            printf("\nCannot find space for word");
            exit(0);
        }
    }

  2. #2
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,538
    Looks good, but I would not make such an implementation. Most implementations allows the caller to pass in a buffer and size, so that you do not need to allocate memory via malloc, thus freeing the responsibility to call free.

    And calling exit in functions is taboo. You should return an error instead and the caller should handle it.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  3. #3
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,047
    Looks alright and if you really want to get into pointers you might want to write your own routine to copy *newWord to *word instead of using strlcpy() to do the work for you.
    Anyways just my .02

  4. #4
    Jack of many languages Dino's Avatar
    Join Date
    Nov 2007
    Location
    Katy, Texas
    Posts
    2,309
    You really don't need:
    Code:
        word = NULL;
    since malloc sets it.
    Mac and Windows cross platform programmer. Ruby lover.

    Quote of the Day
    12/20: Mario F.:I never was, am not, and never will be, one to shut up in the face of something I think is fundamentally wrong.

    Amen brother!

  5. #5
    Registered User officedog's Avatar
    Join Date
    Oct 2008
    Posts
    77
    Thank you for the replies, very helpful as always

    Elysia - I'm not sure exactly what you mean. Is it that it is better to use the stack rather than heap? The direction I'm wanting to go here is eventually to implement a dictionary struct (or possiby class) which contains an array of simpler word structs. Previously I used arrays for each word, but I thought this might be a more 'memory efficient'. I'll have a bit more of a think about this

    I also get the idea of providing a buffer and length, but I was trying to make the function do all that work internally. Any chance of a simple example?

    Dino - I took out NULL, although somewhere I read something about it being a good thing to initialise pointers to NULL

    itCbitC - How does this look?

    Code:
    void ODStrCpy(char *dest, const char *src, int length)
    {
        for (int i = 0; i<length; i++) 
        {
            if (i == length-1)
            {
                dest[i] = '\0';
            }
            
            else if (i< length-1)
            {
                dest[i] = src[i];
            }
        }
    }

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,606
    Quote Originally Posted by officedog
    I took out NULL, although somewhere I read something about it being a good thing to initialise pointers to NULL
    Yes, if you do not have an appropriate address to assign to a pointer, then initialising the pointer to be a null pointer is good practice. In this case, you are going to assign it the return value of malloc on the next line, so setting the pointer to NULL is pointless.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
        word = (char *) malloc(length);
    Since this is C, and not C++, the cast is pointless and may hide other problems. Remove it.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  8. #8
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,538
    Quote Originally Posted by matsp View Post
    Code:
        word = (char *) malloc(length);
    Since this is C, and not C++, the cast is pointless and may hide other problems. Remove it.

    --
    Mats
    If it hides problems, then you have a bad compiler. The cast shouldn't hurt.

    Quote Originally Posted by officedog View Post
    Elysia - I'm not sure exactly what you mean. Is it that it is better to use the stack rather than heap? The direction I'm wanting to go here is eventually to implement a dictionary struct (or possiby class) which contains an array of simpler word structs. Previously I used arrays for each word, but I thought this might be a more 'memory efficient'. I'll have a bit more of a think about this
    What I mean is that when you call the function, the code that calls it perhaps have a proper buffer to use, whether that is on the stack or not, that's irrelevant.
    So then the caller can pass in that buffer to the function and it uses it as a buffer to store the result. This is the common C way.
    The advantages is that the caller can just make a buffer on the stack if it wishes and thus not need to call free later. It decreases complexity. If the caller decides it is necessary, it can also allocate space with malloc and pass it along as the buffer. So you get flexibility.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  9. #9
    Registered User officedog's Avatar
    Join Date
    Oct 2008
    Posts
    77
    Thanks for comments laserlight and matsp. NULL and (char*) cast are deleted now.

    I've been working away on this and it has evolved a little more. I have a sense that I've made a mistake here, but it is closer to the direction I want to go in.

    I have a struct, and an initialisation function "void wordNew". My uncertainty is over whether I should free the temporary pointer (last 2 lines of the function).

    In the function I copy the address to the struct's char pointer. My sense from what I have read is that I should ("free what you malloc"), but I am concerned that by freeing the memory , it will get overwritten.

    main.c
    Code:
    int main (int argc, const char * argv[]) {
        Word w;
        wordNew(&w, "Hello");
    
        return 0;
    }
    word.h
    Code:
    typedef struct {
        char *word;
        int length;
        int isAllocated;
    }Word;
        
    void wordNew(Word *wordStruct, const char * newWord);
    word.c
    Code:
    void wordNew(Word *wordStruct, const char * newWord) {
        int isAllocated;
        char* word;
        int length;
        
        length = strlen(newWord) +1;
        word = malloc(length); //sizeof(char) = 1 byte
    
        if (word != NULL) {
            isAllocated = 1;
            strlcpy(word, newWord, length); //strlcpy adds '\0'
            
            wordStruct->word = word;
            wordStruct->length = length;
            wordStruct->isAllocated = isAllocated;
        }
        else {
            isAllocated = 0;
        }
    
        Are either of these necessary??
        free(word);  //  
        word = NULL;  //is this over-cautious??? 
    }

  10. #10
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,490
    > If it hides problems, then you have a bad compiler. The cast shouldn't hurt.
    The problem it hides is your failure to prototype malloc properly, by including stdlib.h

    Unfortunately, C89 doesn't mandate prototypes, so the "implicit int" problem would arise, and the existance of the cast masks it.

    C99 and C++ have no such implicit rule, so there it isn't a problem.
    Though needing to cast malloc in C++ is a different story altogether.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  11. #11
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,538
    You must not free word inside the function, because then it will not be valid outside.
    You copy that pointer by assigning it in that struct you pass in.
    So it is main's responsibility to free w.word.

    Quote Originally Posted by Salem View Post
    > If it hides problems, then you have a bad compiler. The cast shouldn't hurt.
    The problem it hides is your failure to prototype malloc properly, by including stdlib.h

    Unfortunately, C89 doesn't mandate prototypes, so the "implicit int" problem would arise, and the existance of the cast masks it.

    C99 and C++ have no such implicit rule, so there it isn't a problem.
    Though needing to cast malloc in C++ is a different story altogether.
    I know it does, and it's sad. However, a good compiler will know and warn you about the call of a function without a prototype.
    Visual C++ does, and I am sure others should too.

    I am just pointing out that. I do not recommend adding casts, but I do not recommend removing them, either.
    Last edited by Elysia; 11-06-2008 at 12:23 PM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  12. #12
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Relying on a compiler warning you for doing something wrong is not reliable. It's 10x more likely to warn if you "try to convert integer to pointer" than it is if you have a cast. Some compilers complain about no prototype, others don't. If you are unlucky enough that the code works OK on one system with no warning or failure (perhaps because stdio.h includes stdlib.h), and then you move to another compiler/OS or whatever, the code doesn't work , I promise you that a warning to tell you that "this may not work" is much better.

    There is absolutely no benefit in having a cast on malloc. The only time it's needed is if you compile C-code in a C++ compiler.

    A point to notice about compilers and warnings: when we "integrate" a project to the main branch, we often get warnings becuase the developers in the project have compiled the code only with the "desktop" compiler, not the embedded compiler. The latter gives warnings for some things that the desktop compiler doesn't warn about (unused variables is a typical case). This stops the "build" from being acceptable to the main branch, so it needs fixing.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  13. #13
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,538
    And even better is compiling in strict C99 mode.
    I don't like the idea of non-type strictness. It is alright in languages that has one type to support everything (ie PHP, javascript, etc), but not in languages like C where assigning a non-related type to something else is a bad thing™. That is what casts are for.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  14. #14
    Banned master5001's Avatar
    Join Date
    Aug 2001
    Location
    Visalia, CA, USA
    Posts
    3,685
    Perhaps, for Elysia's sake one could write something like this

    Code:
    #ifdef __cplusplus
    # define MALLOC_CAST(type, statement)  reinterpret_cast< type >( statement )
    #else
    # define MALLOC_CAST(type, statement) statement
    #endif
    
    #define t_malloc (type, size)  MALLOC_CAST(type, malloc( size ))

  15. #15
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,538
    OK, I'm doing whining. I did get my point across.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Quick Question: 2D Array with char **
    By Phoenix_Rebirth in forum C Programming
    Replies: 4
    Last Post: 01-29-2009, 06:33 AM
  2. malloc question
    By adrive in forum C Programming
    Replies: 11
    Last Post: 07-24-2008, 09:27 AM
  3. Question on malloc() and reading strings
    By rockysfr in forum C Programming
    Replies: 2
    Last Post: 09-04-2005, 06:37 PM
  4. how to use malloc for array of strings
    By SoFarAway in forum C Programming
    Replies: 3
    Last Post: 02-18-2005, 03:19 PM
  5. malloc() strings VS array strings
    By Kleid-0 in forum C Programming
    Replies: 5
    Last Post: 01-10-2005, 09:26 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21