Returning a string array from a function

Discussion in 'C Programming' started by Adam, Sep 2, 2007.

  1. Adam

    Adam Guest

    Hi,

    I'd like to return an (arbitrary length) string array from a function so
    that after calling the array I've got a list of strings I can access.

    After much perusing of the internet I found a related answer here (by
    Eric Sosman) which involved creating an array of pointers and using
    that, so it looks something like:


    void foo(char *sptr[], int items)
    {
    char buff[50];
    while (--items >= 0)
    {
    sprintf (buff, "foo%d", items);
    sptr[items] = malloc(strlen(buff) + 1);
    if (sptr[items] == NULL)
    exit(0);
    strcpy (sptr[items], buff);
    }
    }

    int main(void)
    {
    int total = 10;
    char *p[total];

    foo(p, total);

    printf(p[1]);

    return 0;
    }


    This works a treat (I can access my strings, e.g. the printf line) but
    how would I do a similar thing if I don't know how long my list is going
    to be? E.g. "total" only gets found out inside foo.

    Thanks a lot,
    Adam

    --
    Adam Richardson
    Carpe Diem
     
    Adam, Sep 2, 2007
    #1
    1. Advertising

  2. "Adam" <> wrote in message
    news:...
    > Hi,
    >
    > I'd like to return an (arbitrary length) string array from a function so
    > that after calling the array I've got a list of strings I can access.
    >
    > After much perusing of the internet I found a related answer here (by
    > Eric Sosman) which involved creating an array of pointers and using
    > that, so it looks something like:
    >
    >
    > void foo(char *sptr[], int items)
    > {
    > char buff[50];
    > while (--items >= 0)
    > {
    > sprintf (buff, "foo%d", items);
    > sptr[items] = malloc(strlen(buff) + 1);
    > if (sptr[items] == NULL)
    > exit(0);
    > strcpy (sptr[items], buff);
    > }
    > }
    >
    > int main(void)
    > {
    > int total = 10;
    > char *p[total];
    >
    > foo(p, total);
    >
    > printf(p[1]);
    >
    > return 0;
    > }
    >
    >
    > This works a treat (I can access my strings, e.g. the printf line) but
    > how would I do a similar thing if I don't know how long my list is going
    > to be? E.g. "total" only gets found out inside foo.
    >
    > Thanks a lot,
    > Adam
    >

    char **foo(int items)
    {
    char **answer;
    char buff[100];
    int i;

    answer = malloc(items * sizeof(char *));
    if(!answer)
    goto error_exit;
    /* needed for clean up, mustn't have non-null garbage pointers */
    for(i=0;i<items;i++)
    answer = 0;
    for(i=0;i<items;i++)
    {
    sprintf(buff, "foo %d", i):
    answer = malloc(stren(buff) + 1);
    if(!answer)
    goto error_exit;
    strcpy(answer, buff);
    }
    return answer;
    error_exit:
    if(answer)
    for(i=0;i<items;i++)
    free(answer);
    free(answer);
    /* maybe print error message / terminate here */
    return 0;
    }
     
    Malcolm McLean, Sep 2, 2007
    #2
    1. Advertising

  3. Adam

    Eric Sosman Guest

    Adam wrote:
    > Hi,
    >
    > I'd like to return an (arbitrary length) string array from a function so
    > that after calling the array I've got a list of strings I can access.


    First problem: functions cannot return arrays. A fairly
    close alternative is to use a function that returns a pointer
    to something, with the tacit understanding that the pointed-to
    something is the first one in an array of somethings.

    Why is the distinction important? Because the pointer just
    points to one single something, and conveys no information about
    how many additional somethings might follow it. What *that*
    means is that you need some other way to communicate the count.
    Two popular approaches are to give the function an extra argument
    that points to a place where it can store the count, or to store
    a special value (NULL, perhaps) in an array slot after all the
    "payload" positions, in the same way the end of a string is
    marked by an extra '\0' character.

    > After much perusing of the internet I found a related answer here (by
    > Eric Sosman) which involved creating an array of pointers and using
    > that, so it looks something like:
    >
    >
    > void foo(char *sptr[], int items)
    > {
    > char buff[50];
    > while (--items >= 0)
    > {
    > sprintf (buff, "foo%d", items);
    > sptr[items] = malloc(strlen(buff) + 1);
    > if (sptr[items] == NULL)
    > exit(0);
    > strcpy (sptr[items], buff);
    > }
    > }
    >
    > int main(void)
    > {
    > int total = 10;
    > char *p[total];


    Are you using a C99 compiler? Prior to C99, array
    dimensions had to be compile-time constants, but `total'
    is a variable.

    > foo(p, total);
    >
    > printf(p[1]);
    >
    > return 0;
    > }
    >
    >
    > This works a treat (I can access my strings, e.g. the printf line) but
    > how would I do a similar thing if I don't know how long my list is going
    > to be? E.g. "total" only gets found out inside foo.


    One way is to proceed more or less as above, but to
    have foo() tell its caller how many of the sptr[] slots
    actually got filled in; this number might be the value of
    the function foo(), or you could use a NULL "sentinel" as
    mentioned above. If you do this, the meaning of the second
    argument should change from "make N strings" to "sptr[] has
    at least N slots, but perhaps no more." This allows foo()
    to avoid storing past the end of the sptr[] array if `total'
    turns out to be larger than the caller anticipated.

    Another way is for foo() to allocate the sptr "array" by
    itself, using malloc(). If foo() later discovers that the
    allocation was too small, it can use realloc() to make it
    larger. When foo() is finished, it returns a pointer to the
    start of the dynamically-allocated sptr "array" (and uses
    communicates the count by some other means).

    A variation on the second theme may be simpler if foo()
    can discover `total' in one pass, then malloc() an sptr
    "array" of what is now known to be the correct size, then
    make a second pass to fill in the elements.

    The latter two methods share one important feature (or
    drawback): Since foo() allocates memory dynamically and leaves
    it allocated when it returns, the responsibility for free()ing
    the memory falls on someone else. This needs to be prominently
    mentioned in the documentation for foo(), lest unwary callers
    leak scads of memory by calling foo(), using its results for a
    while, and then just "dropping them on the floor."

    --
    Eric Sosman
    lid
     
    Eric Sosman, Sep 2, 2007
    #3
  4. In article <>,
    Malcolm McLean <> wrote:

    >"Adam" <> wrote in message
    >news:...


    >> I'd like to return an (arbitrary length) string array from a function so
    >> that after calling the array I've got a list of strings I can access.


    > answer = malloc(items * sizeof(char *));
    > if(!answer)
    > goto error_exit;
    > /* needed for clean up, mustn't have non-null garbage pointers */


    Hmmm. Adam's question didn't -sound- like textbook homework, that you
    needed to use a goto to down-grade the code quality so as to
    down-grade the marks of anyone who copied it literally.
    --
    I was very young in those days, but I was also rather dim.
    -- Christopher Priest
     
    Walter Roberson, Sep 2, 2007
    #4
  5. Adam

    Adam Guest

    In message <>, Eric Sosman
    wrote:
    > Adam wrote:
    > > I'd like to return an (arbitrary length) string array from a function so
    > > that after calling the array I've got a list of strings I can access.

    ^^^^^ function

    [snip discussion]

    Thanks a lot for the helpful info. I'll go away and do some pondering
    about the best course of action. :)

    > Are you using a C99 compiler?


    Yep, GCC, -std=c99.

    Thanks,
    Adam

    --
    Adam Richardson
    Carpe Diem
     
    Adam, Sep 2, 2007
    #5
  6. Adam <> writes:
    > In message <>, Eric Sosman
    > wrote:

    [...]
    >> Are you using a C99 compiler?

    >
    > Yep, GCC, -std=c99.


    Then you're using a compiler that implements most, but not all, of C99.
    See <http://gcc.gnu.org/c99status.html>.

    --
    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."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Sep 2, 2007
    #6
  7. Adam

    Adam Guest

    In message <>, Eric Sosman
    wrote:

    > Adam wrote:


    > > I'd like to return an (arbitrary length) string array from a
    > > function so that after calling the array I've got a list of strings
    > > I can access.

    [snip]
    > > After much perusing of the internet I found a related answer here
    > > (by Eric Sosman) which involved creating an array of pointers and
    > > using that, so it looks something like:
    > >
    > > void foo(char *sptr[], int items)
    > > {
    > > char buff[50];
    > > while (--items >= 0)
    > > {
    > > sprintf (buff, "foo%d", items);
    > > sptr[items] = malloc(strlen(buff) + 1);
    > > if (sptr[items] == NULL)
    > > exit(0);
    > > strcpy (sptr[items], buff);
    > > }
    > > }
    > > [but how can I do it without specifying "items"?]

    [snip]
    >
    > Another way is for foo() to allocate the sptr "array" by
    > itself, using malloc().


    OK, I've been thinking a bit more about this and trying to come to terms
    with pointers to pointers and malloc - neither of which I've used
    before! However, I'm stuck.

    Thanks to Malcolm, who's example I've butchered to try and get working
    (I've removed the error-checking etc to try and get it down to
    fundamentals, but I realise that's important):

    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>

    void foo(char **ptr)
    {
    char **answer;
    char buff[100];
    int i;
    int items = 10;

    answer = malloc(items * sizeof(char *));

    for(i=0;i<items;i++)
    {
    sprintf(buff, "foo %d", i);
    answer = malloc(strlen(buff) + 1);
    strcpy(answer, buff);
    }

    printf(answer[2]);

    ptr = answer;
    }

    int main(void)
    {
    char *listofptrs;

    foo(&listofptrs);

    printf("5) %s",listofptrs[5]);

    return 0;
    }

    answer[2] gets displayed OK, but everything goes nasty before
    listofptrs[5] (and the compiler spouts warnings about the printf line
    not being right).

    On the face of it, since listofptrs[5] is a pointer to the string, I'd
    have thought I ought to use *listofptrs[5] but I get an error about
    misuse of "unary *".

    I'm sure I'm doing something stupid wrong, but I can't work it out. Any
    pointers welcome! ;)

    Thanks,
    Adam

    --
    Adam Richardson
    Carpe Diem
     
    Adam, Sep 5, 2007
    #7
  8. Adam

    Default User Guest

    Adam wrote:


    > OK, I've been thinking a bit more about this and trying to come to
    > terms with pointers to pointers and malloc - neither of which I've
    > used before! However, I'm stuck.


    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > void foo(char **ptr)
    > {
    > char **answer;
    > char buff[100];
    > int i;
    > int items = 10;
    >
    > answer = malloc(items * sizeof(char *));
    >
    > for(i=0;i<items;i++)
    > {
    > sprintf(buff, "foo %d", i);
    > answer = malloc(strlen(buff) + 1);
    > strcpy(answer, buff);
    > }
    >
    > printf(answer[2]);
    >
    > ptr = answer;


    ptr is a local pointer. Nothing you do to it in foo() will be reflected
    back in main().


    > }
    >
    > int main(void)
    > {
    > char *listofptrs;


    In spite of the name, listofptrs is a single pointer to char. NOT what
    you want.

    > foo(&listofptrs);
    >
    > printf("5) %s",listofptrs[5]);


    listofptrs[5] would be at best a single char, not a string.

    > return 0;
    > }


    I wouldn't do it this way for real, but here's a way with the least
    amount of fiddling with your example that achieves the goal:

    #include <stdio.h>
    #include <string.h>
    #include <stdlib.h>

    void foo(char ***ptr)
    {
    char **answer;
    char buff[100];
    int i;
    int items = 10;

    answer = malloc(items * sizeof(char *));

    for(i=0;i<items;i++)
    {
    sprintf(buff, "foo %d", i);
    answer = malloc(strlen(buff) + 1);
    strcpy(answer, buff);
    }

    printf(answer[2]);

    *ptr = answer;
    }

    int main(void)
    {
    char **listofptrs;

    foo(&listofptrs);

    printf("5) %s",listofptrs[5]);

    return 0;
    }



    Brian
     
    Default User, Sep 5, 2007
    #8
  9. On 2007-09-05, Adam <> wrote:
    > OK, I've been thinking a bit more about this and trying to come to terms
    > with pointers to pointers and malloc - neither of which I've used
    > before! However, I'm stuck.
    >
    > Thanks to Malcolm, who's example I've butchered to try and get working
    > (I've removed the error-checking etc to try and get it down to
    > fundamentals, but I realise that's important):
    >
    > #include <stdio.h>
    > #include <string.h>
    > #include <stdlib.h>
    >
    > void foo(char **ptr)
    > {
    > char **answer;
    > char buff[100];
    > int i;
    > int items = 10;
    >
    > answer = malloc(items * sizeof(char *));
    >
    > for(i=0;i<items;i++)
    > {
    > sprintf(buff, "foo %d", i);
    > answer = malloc(strlen(buff) + 1);
    > strcpy(answer, buff);
    > }
    >
    > printf(answer[2]);
    >
    > ptr = answer;
    > }
    >
    > int main(void)
    > {
    > char *listofptrs;
    >
    > foo(&listofptrs);
    >
    > printf("5) %s",listofptrs[5]);
    >
    > return 0;
    > }
    >
    > answer[2] gets displayed OK, but everything goes nasty before
    > listofptrs[5] (and the compiler spouts warnings about the printf line
    > not being right).
    >
    > On the face of it, since listofptrs[5] is a pointer to the string, I'd
    > have thought I ought to use *listofptrs[5] but I get an error about
    > misuse of "unary *".

    [snip]

    Given the declaration

    char *listofptrs"

    listofptrs[5] is a char. To have listofptrs[5] be a char*, you can use
    the declaration

    char **listofptrs

    With that change to the declaration, you need to change

    void foo(char **ptr)

    to

    void foo(char ***ptr)

    and inside the function foo change

    ptr = answer;

    to

    *ptr = answer;
     
    A. Bolmarcich, Sep 5, 2007
    #9
  10. Adam

    Adam Guest

    On 5 Sep, 23:36, "Default User" <> wrote:

    > ptr is a local pointer. Nothing you do to it in foo() will be reflected
    > back in main().


    Of course. I'd had a feeling there was something wrong with that
    expression. If I'd nailed it down I might have solved my problem! :)

    > I wouldn't do it this way for real, but here's a way with the least


    Any alternative suggestions are welcome :)

    Thanks,
    Adam
     
    Adam, Sep 6, 2007
    #10
  11. Adam

    Default User Guest

    Adam wrote:

    > On 5 Sep, 23:36, "Default User" <> wrote:
    >
    > > ptr is a local pointer. Nothing you do to it in foo() will be
    > > reflected back in main().

    >
    > Of course. I'd had a feeling there was something wrong with that
    > expression. If I'd nailed it down I might have solved my problem! :)
    >
    > > I wouldn't do it this way for real, but here's a way with the least

    >
    > Any alternative suggestions are welcome :)


    It's poor practice to use a magic number defined in the worker
    function. How would the caller know what the value of "items" is?

    One thing to consider is whether you want to do all the allocating in a
    separate function. The caller then has to deallocate it.

    If you do, then you at least need to tell the caller how many items
    have been indicated. Either a return value or another parameter is the
    way to do that.



    Brian
     
    Default User, Sep 6, 2007
    #11
  12. Adam

    Adam Guest

    In message <>, Default User wrote:

    > Adam wrote:
    >
    > > On 5 Sep, 23:36, "Default User" <> wrote:
    > >
    > > > ptr is a local pointer. Nothing you do to it in foo() will be
    > > > reflected back in main().

    > >
    > > Of course. I'd had a feeling there was something wrong with that
    > > expression. If I'd nailed it down I might have solved my problem! :)
    > >
    > > > I wouldn't do it this way for real, but here's a way with the least

    > >
    > > Any alternative suggestions are welcome :)

    >
    > It's poor practice to use a magic number defined in the worker
    > function. How would the caller know what the value of "items" is?


    Sorry, I should have said - that's only for illustration. Assigning the
    actual strings will be done by an OS-specific routine. Also, I need to
    think about error checking and providing a function to free the memory.

    Adam

    --
    Adam Richardson
    Carpe Diem
     
    Adam, Sep 6, 2007
    #12
  13. On 5 Sep 2007 at 22:36, Default User wrote:
    > ptr is a local pointer. Nothing you do to it in foo() will be reflected
    > back in main().


    Gasp! Default Loser actually makes a post to clc with technical content,
    rather than a moan about top-posting or an accusation of trolling!
    *falls off chair in amazement*
     
    Antoninus Twink, Sep 7, 2007
    #13
  14. On Wed, 05 Sep 2007 23:12:12 +0100, Adam <>
    wrote:


    snip previous code not part of this discussion

    >OK, I've been thinking a bit more about this and trying to come to terms
    >with pointers to pointers and malloc - neither of which I've used
    >before! However, I'm stuck.
    >
    >Thanks to Malcolm, who's example I've butchered to try and get working
    >(I've removed the error-checking etc to try and get it down to
    >fundamentals, but I realise that's important):
    >
    >#include <stdio.h>
    >#include <string.h>
    >#include <stdlib.h>
    >
    >void foo(char **ptr)
    >{
    > char **answer;
    > char buff[100];
    > int i;
    > int items = 10;
    >
    > answer = malloc(items * sizeof(char *));
    >
    > for(i=0;i<items;i++)
    > {
    > sprintf(buff, "foo %d", i);
    > answer = malloc(strlen(buff) + 1);
    > strcpy(answer, buff);
    > }
    >
    > printf(answer[2]);
    >
    > ptr = answer;
    >}


    There are very few ways for the activities of a function to be
    reflected in the calling program.

    1 - Use external or file scope variables (sometimes called
    global variables) that the calling function will also have access to.
    This is seldom the best approach.

    2 - Have the function return the value of an object which
    contains the "results".

    3 - Pass the function the address of an object into which the
    function can store the "results". This approach is more frequently
    used when the return value is already in use (look at strtol for an
    example).

    As others have noted, your function does none of the above so the data
    pointed to by answer and the data pointed to by the pointers in answer
    are all lost when foo returns.

    To use approach 3, you would declare the function as accepting
    a char*** argument, call it from main specifying &listofptrs (after
    changing that to a char**), and replacing the last statement in foo
    with
    *ptr = answer;

    A better (tm) approach would be to change the function return
    to char**, delete the parameter, and change the last statement to

    return answer;
    You still need to change listofptrs in main but then you call the
    function with

    listofptrs = foo();

    >
    >int main(void)
    >{
    > char *listofptrs;


    A char* must point to a single char. That char can be the first in a
    sequence which can be treated as an array of char.

    If you want something to point to a char* (which may also be treated
    as the first in a sequence or first of an array), you need a pointer
    to char*. Substituting the normal verbal definition, you need a
    pointer to pointer to char, coded as char**.

    >
    > foo(&listofptrs);
    >
    > printf("5) %s",listofptrs[5]);
    >
    > return 0;
    >}
    >
    >answer[2] gets displayed OK, but everything goes nasty before
    >listofptrs[5] (and the compiler spouts warnings about the printf line
    >not being right).


    Say thank you to the compiler writer. He did you a favor, above and
    beyond the requirements of the standard. As you have coded it,
    listofptrs is a char*. listofptrs[5] must therefore be a char. But
    %s requires an char* for the corresponding argument. This mismatch
    invokes undefined behavior during execution. "Going nasty" is
    actually one of the more preferable manifestations of undefined
    behavior.

    >
    >On the face of it, since listofptrs[5] is a pointer to the string, I'd


    But it's not, as you can now recognize.

    >have thought I ought to use *listofptrs[5] but I get an error about
    >misuse of "unary *".


    The dereference operator (unary *) can only be applied to a pointer
    (but not to a void*). Since listofptrs[5] is not a pointer, this is a
    constraint violation that requires a diagnostic.


    Remove del for email
     
    Barry Schwarz, Sep 7, 2007
    #14
  15. Adam

    Adam Guest

    [to recap, I'm trying to generate an artibrary list of strings in a
    function for use in main()]

    In message <>, Adam wrote:

    > Sorry, I should have said - that's only for illustration. Assigning the
    > actual strings will be done by an OS-specific routine. Also, I need to
    > think about error checking and providing a function to free the memory.


    OK, I've got my function working now but am still a bit uncertain about
    my use of free, so here's my final attempt - I'd be grateful for any
    comments :)

    It's got some psuedo-code in it to replace the bits where my real
    function has OS-specific bits. The return-value of the real function is
    also a slightly more sophisticated error structure.


    int create_list(char ***ptrlist)
    {
    char **list;
    char *name;
    int items, i, count = 0;

    items = /* number determined by OS routine */ ;

    /* Allocate space for a list of pointers long enough */
    /* for all items + terminating "" */
    list = malloc((items + 1) * sizeof(char *));
    if (!list)
    return 1;

    while ( /* more items */ )
    {
    /* pointer to an item string written to "name" by OS routine here */

    /* Allocate length of string + terminator */
    list[count] = malloc(strlen(name) + 1);

    if (!list[count])
    {
    /* Free already-allocated items */
    for (i = 0; i < count; i++)
    free(list[count]);

    return 1;
    }

    if ( /* last item */ )
    strcpy(list[count], ""); /* Terminate list with "" */
    else
    strcpy(list[count], name);

    count++;
    }

    *ptrlist = list;

    return 0;
    }

    void clear_list(char **ptrlist)
    {
    int i = 0;

    /* Free the strings pointed to in the pointer list */
    while(strcmp(ptrlist, ""))
    {
    free(ptrlist);
    i++;
    }

    /* Free the pointer list */
    free(ptrlist);
    }

    int main(void)
    {
    char **listofptrs;
    int i = 0;

    create_list(&listofptrs); /* Create list */

    /* Use list, e.g.: */
    while(strcmp(ptrlist, ""))
    {
    printf(ptrlist);
    i++;
    }

    clear_list(listofptrs); /* Free memory */

    return 0;
    }


    On an unrelated note, what's the best practice when it comes to using
    "return"? Up until now I've always only had one return from a function,
    but create_list has a few places where return is called...

    Thanks a lot,
    Adam

    --
    Adam Richardson
    Carpe Diem
     
    Adam, Sep 8, 2007
    #15
  16. Adam

    Chris Dollin Guest

    Adam wrote:

    > On an unrelated note, what's the best practice when it comes to using
    > "return"? Up until now I've always only had one return from a function,
    > but create_list has a few places where return is called...


    I don't know what "best practice" is, but I do have opinions.

    (a) If you have a local coding standard, stick to it unless it's
    so horrible you're prepared to advocate for change.

    (b) Code being /clear/ trumps any blind application of rules.

    (c) If a function is so long that it's hard to see where any
    extra returns are, that function is anyways too long. Fix
    that first.

    (d) Code being /clear/ trumps any blind application of rules.

    (e) Code should say what it means and mean what it says.

    (f) If a function has blank lines in it, it's too long, unless
    it has a switch in it, in which case it /might/ be too long.

    I freely write functions with multiple returns in them, because
    I follow (b) and (c) and nobody has tried to impose an unsuitable (a).
    For example I'd write (I'm not advocating this brace placement [1])

    if (whatever)
    {
    thenWhatever();
    return answerA;
    }
    else
    {
    elseWhatever();
    return answerB;
    }

    rather than any sort of dance with a variable to hold the return
    value. That's assuming, of course, that I can't write it like:

    return whatever ? answerAafterThen : answerBafterElse;

    which would be my preference, subject to (b).

    [1] I don't lie well, do I.

    --
    Trumps Are Green And Sevens Hedgehog
    "It took a very long time, much longer than the most generous estimates."
    - James White, /Sector General/
     
    Chris Dollin, Sep 8, 2007
    #16
  17. Adam <> writes:

    > [to recap, I'm trying to generate an artibrary list of strings in a
    > function for use in main()]
    >
    > In message <>, Adam wrote:
    >
    >> Sorry, I should have said - that's only for illustration. Assigning the
    >> actual strings will be done by an OS-specific routine. Also, I need to
    >> think about error checking and providing a function to free the memory.

    >
    > OK, I've got my function working now but am still a bit uncertain about
    > my use of free, so here's my final attempt - I'd be grateful for any
    > comments :)


    Not quite right yet...

    > It's got some psuedo-code in it to replace the bits where my real
    > function has OS-specific bits. The return-value of the real function is
    > also a slightly more sophisticated error structure.
    >
    >
    > int create_list(char ***ptrlist)
    > {
    > char **list;
    > char *name;
    > int items, i, count = 0;
    >
    > items = /* number determined by OS routine */ ;
    >
    > /* Allocate space for a list of pointers long enough */
    > /* for all items + terminating "" */


    It would be more idiomatic to use NULL for the final sentinel value
    and even more idiomatic to return the number of strings and have no
    sentinel. By using "" you are have to either allocate a space for
    this short string, or you can use a pointer to a constant "" but both
    of these are more complex than using NULL.

    > list = malloc((items + 1) * sizeof(char *));
    > if (!list)
    > return 1;
    >
    > while ( /* more items */ )
    > {
    > /* pointer to an item string written to "name" by OS routine here */
    >
    > /* Allocate length of string + terminator */
    > list[count] = malloc(strlen(name) + 1);
    >
    > if (!list[count])
    > {
    > /* Free already-allocated items */
    > for (i = 0; i < count; i++)
    > free(list[count]);


    Typo? You mean free(list), yes?

    Here you should probably also free(list) since it is not required
    now. If you used NULL as a terminator, it would be already terminated
    (the malloc just failed) and you could just call clear_list to throw
    everything away!

    >
    > return 1;
    > }
    >
    > if ( /* last item */ )
    > strcpy(list[count], ""); /* Terminate list with "" */


    I would prefer to see this outside the loop. It makes more sense to
    me that way. Also, it is not clear if you will have make space for
    this short string. If strlen(name) returns 0 on the last iteration
    then you are OK.

    > else
    > strcpy(list[count], name);
    >
    > count++;
    > }
    >
    > *ptrlist = list;
    >
    > return 0;
    > }
    >
    > void clear_list(char **ptrlist)
    > {
    > int i = 0;
    >
    > /* Free the strings pointed to in the pointer list */
    > while(strcmp(ptrlist, ""))


    prtlist[0] == '\0' or even !*prtlist[0] are simpler and idiomatic
    in C.

    > {
    > free(ptrlist);
    > i++;
    > }
    >
    > /* Free the pointer list */
    > free(ptrlist);
    > }
    >
    > int main(void)
    > {
    > char **listofptrs;
    > int i = 0;
    >
    > create_list(&listofptrs); /* Create list */
    >
    > /* Use list, e.g.: */
    > while(strcmp(ptrlist, ""))


    This test would be simpler with a NULL as the end marker.

    > {
    > printf(ptrlist);
    > i++;
    > }
    >
    > clear_list(listofptrs); /* Free memory */
    >
    > return 0;
    > }
    >
    >
    > On an unrelated note, what's the best practice when it comes to using
    > "return"? Up until now I've always only had one return from a function,
    > but create_list has a few places where return is called...


    Pass -- I'll leave that can of worms unopened!

    --
    Ben.
     
    Ben Bacarisse, Sep 8, 2007
    #17
  18. Chris Dollin wrote:

    > Adam wrote:
    >
    >> On an unrelated note, what's the best practice when it comes to using
    >> "return"? Up until now I've always only had one return from a
    >> function, but create_list has a few places where return is called...

    >
    > I don't know what "best practice" is, but I do have opinions.
    >

    <snip>
    > (f) If a function has blank lines in it, it's too long, unless
    > it has a switch in it, in which case it /might/ be too long.


    I don't agree on this one.
    I regularly put blank lines between logical sections in my code such as
    variable declarations (C90 style, at beginning of a block),
    pre-condition checks and the code that does the actual work.

    However, if you can split the actual work into multiple logical units,
    then it is highly likely that your function is doing too much and is
    therefor too long.

    Bart v Ingen Schenau
    --
    a.c.l.l.c-c++ FAQ: http://www.comeaucomputing.com/learn/faq
    c.l.c FAQ: http://www.eskimo.com/~scs/C-faq/top.html
    c.l.c++ FAQ: http://www.parashift.com/c -faq-lite/
     
    Bart van Ingen Schenau, Sep 9, 2007
    #18
  19. Adam

    Army1987 Guest

    On Sat, 08 Sep 2007 16:25:07 +0100, Ben Bacarisse wrote:

    >> while(strcmp(ptrlist, ""))

    >
    > This test would be simpler with a NULL as the end marker.

    I agree that it is silly to use an empty string as a sentinel, but
    how is while(ptrlist) simpler than while(ptrlist[0])?
    --
    Army1987 (Replace "NOSPAM" with "email")
    If you're sending e-mail from a Windows machine, turn off Microsoft's
    stupid “Smart Quotes†feature. This is so you'll avoid sprinkling garbage
    characters through your mail. -- Eric S. Raymond and Rick Moen
     
    Army1987, Sep 9, 2007
    #19
  20. Adam

    CBFalconer Guest

    Army1987 wrote:
    > Ben Bacarisse wrote:
    >
    >>> while(strcmp(ptrlist, ""))

    >>
    >> This test would be simpler with a NULL as the end marker.

    >
    > I agree that it is silly to use an empty string as a sentinel,
    > but how is while(ptrlist) simpler than while(ptrlist[0])?


    The rest of the context seems to have been snipped, but on the
    basis of the above it omits at least one level of indirection.
    "while (ptrlist)" needs only to load ptrlist and i, add them,
    and load from the result to test (which may all be one
    instruction). "while (ptrlist[0])" needs to load from that
    result before applying the test.

    --
    Chuck F (cbfalconer at maineline dot net)
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net>


    --
    Posted via a free Usenet account from http://www.teranews.com
     
    CBFalconer, Sep 9, 2007
    #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. Replies:
    11
    Views:
    681
    Christos Georgiou
    May 2, 2006
  2. john
    Replies:
    1
    Views:
    316
    Barry Schwarz
    Jun 29, 2004
  3. Guha

    Returning a 2D array from a function

    Guha, Sep 16, 2005, in forum: C Programming
    Replies:
    1
    Views:
    352
    Barry Schwarz
    Sep 17, 2005
  4. shyam

    function returning array of strings

    shyam, Nov 28, 2005, in forum: C Programming
    Replies:
    6
    Views:
    316
    Serg1976
    Nov 26, 2007
  5. Angus

    a function returning a char array

    Angus, Jan 3, 2007, in forum: C Programming
    Replies:
    9
    Views:
    604
    Jamie Boy
    Feb 7, 2007
Loading...

Share This Page