return char pointer which is malloc'ed in function

Discussion in 'C Programming' started by Khookie, Dec 10, 2007.

  1. Khookie

    Khookie Guest

    Hi everyone

    I'm not sure whether this is good practice or not, so I thought I'll
    ask.

    I've got a function that returns a char pointer, which is malloc'ed in
    the function itself, as per below code. It's basically a singly
    linked list of strings that are concatenated into a big string.

    My question is - since you have to know to free it yourself (usage
    below), is this considered bad practice? Or should the user of the
    function know?

    Function:
    char* string_list_concat(string_list *list) {
    char* result = malloc(list->length + 1);
    char* result_ptr = result;

    // Create string
    string_node *node = list->bol;
    while (node != list->eol) {
    char* string_node_ptr = node->string;
    while(*string_node_ptr != '\0') {
    *result_ptr = *string_node_ptr;
    string_node_ptr++;
    result_ptr++;
    }
    node = node->next;
    }
    *result_ptr = '\0';
    return result;
    }

    Usage:
    char* result = string_list_concat(list);
    printf("Output is: %s\n", result);
    free(result);

    Cheers

    Chris
    Khookie, Dec 10, 2007
    #1
    1. Advertising

  2. Khookie

    Chris Dollin Guest

    Khookie wrote:

    > I've got a function that returns a char pointer, which is malloc'ed in
    > the function itself, as per below code. It's basically a singly
    > linked list of strings that are concatenated into a big string.
    >
    > My question is - since you have to know to free it yourself (usage
    > below), is this considered bad practice? Or should the user of the
    > function know?


    (answer below)

    >
    > Function:
    > char* string_list_concat(string_list *list) {
    > char* result = malloc(list->length + 1);


    That can't be right: I think you have a missing multiplication by
    `sizeof *result`.

    To answer your question:

    (a) the documentation of the function should make it clear who's
    responsible for the storage it allocates.

    (b) don't make it hard to keep track of responsibilities. Sometimes
    you'll want to introduce idioms and functions to help you.

    (c) respect the responsibilities given to you.

    For (b), there are various things you can do to reduce the load, eg:

    * write functions like

    void with_concat_do( string_list *list, void (*it)( string_list * ) )
    {
    string_list * list = concatenated list;
    it( list );
    free( list );
    }

    so that the user /can't/ forget to call `free` (unless an `exit` or
    `longjmp` happens, in which case you're pretty much skewered anyway).

    * pass in an additional store-managing object which is already the
    caller's responsibility and use that to allocate store.

    --
    Chris "or use garbage collection (fx:duck-and-run)" Dollin

    Hewlett-Packard Limited registered no:
    registered office: Cain Road, Bracknell, Berks RG12 1HN 690597 England
    Chris Dollin, Dec 10, 2007
    #2
    1. Advertising

  3. "Chris Dollin" <> schrieb im Newsbeitrag
    news:fjjgci$56g$...
    > Khookie wrote:
    >
    >> I've got a function that returns a char pointer, which is malloc'ed in
    >> the function itself, as per below code. It's basically a singly
    >> linked list of strings that are concatenated into a big string.
    >>
    >> My question is - since you have to know to free it yourself (usage
    >> below), is this considered bad practice? Or should the user of the
    >> function know?

    >
    > (answer below)
    >
    >>
    >> Function:
    >> char* string_list_concat(string_list *list) {
    >> char* result = malloc(list->length + 1);

    >
    > That can't be right: I think you have a missing multiplication by
    > `sizeof *result`.

    Which here is 1 anyway, sizeof(char)...

    Bye, Jojo
    Joachim Schmitz, Dec 10, 2007
    #3
  4. Khookie

    Chris Dollin Guest

    Joachim Schmitz wrote:

    >>> char* string_list_concat(string_list *list) {
    >>> char* result = malloc(list->length + 1);

    >>
    >> That can't be right: I think you have a missing multiplication by
    >> `sizeof *result`.

    > Which here is 1 anyway, sizeof(char)...


    Colour me idiotic: I kept seeing `result` there as `char **`. Thanks, Jojo.
    Apologies to Khookie (and everyone else) for inflicting my Stupidity Of
    The Day [1] on them.

    [1] Please let it be the only one.

    --
    Chris "glasses - check; keys - check; king - check, BOOM" Dollin

    Hewlett-Packard Limited registered no:
    registered office: Cain Road, Bracknell, Berks RG12 1HN 690597 England
    Chris Dollin, Dec 10, 2007
    #4
  5. Khookie

    regis Guest

    Joachim Schmitz wrote:
    > "Chris Dollin" <> schrieb im Newsbeitrag
    > news:fjjgci$56g$...
    >
    >>Khookie wrote:
    >>
    >>
    >>>I've got a function that returns a char pointer, which is malloc'ed in
    >>>the function itself, as per below code. It's basically a singly
    >>>linked list of strings that are concatenated into a big string.
    >>>
    >>>My question is - since you have to know to free it yourself (usage
    >>>below), is this considered bad practice? Or should the user of the
    >>>function know?

    >>
    >>(answer below)
    >>
    >>>Function:
    >>>char* string_list_concat(string_list *list) {
    >>> char* result = malloc(list->length + 1);

    >>
    >>That can't be right: I think you have a missing multiplication by
    >>`sizeof *result`.

    >
    > Which here is 1 anyway, sizeof(char)...


    one would have expected that list->length
    is the number of nodes of the list, instead of
    the sum of strlen() applied to all node->string of the list...
    regis, Dec 11, 2007
    #5
    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. lovecreatesbeauty
    Replies:
    1
    Views:
    1,009
    Ian Collins
    May 9, 2006
  2. davidb
    Replies:
    0
    Views:
    752
    davidb
    Sep 1, 2006
  3. davidb
    Replies:
    6
    Views:
    1,534
    Default User
    Sep 1, 2006
  4. decker
    Replies:
    9
    Views:
    318
    Barry Schwarz
    Nov 10, 2007
  5. Gene
    Replies:
    0
    Views:
    440
Loading...

Share This Page