Returning a string array from a function

[Adam]

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

 

[Malcolm McLean]

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;
}

 

[Eric Sosman]

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."

 

[Walter Roberson]

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.

 

[Adam]

^^^^^ 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

 

[Keith Thompson]

In message <[email protected]>, 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>.

 

[Adam]

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

 

[Default User]

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

 

[A. Bolmarcich]

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;

 

[Adam]

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

 

[Default User]

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!


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

 

[Adam]

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

 

[Antoninus Twink]

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*

 

[Barry Schwarz]

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

 

[Adam]

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

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

 

[Chris Dollin]

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.

 

[Ben Bacarisse]

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

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!

 

[Bart van Ingen Schenau]

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

(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

 

[Army1987]

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])?

 

[CBFalconer]

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.