I tried looking for a solution but wasn't able to find one. Is it possible to return a string? I want to pass a string back from the function below to main. I want to pass the listofdeatils string.
Here's my code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
void readFile();
int main()
{
readFile();
getchar();
printf("%s \n", listofdetails[1]);
getchar();
return 0;
}
void readFile()
{
int i;
FILE *fp;
fp = fopen("specification.txt", "r");
char ** listofdetails;
listofdetails = malloc(sizeof(char*)*6);
for(i=0;i<6;i++)
{
listofdetails[i] = malloc(sizeof(char)*40);
fgets(listofdetails[i], 40, fp);
printf("%s \n", listofdetails[i]);
free(listofdetails[i]);
}
free(listofdetails);
fclose(fp);
}
Try this:
void readFile(char listofdetails[6][40]);
int main()
{
char listofdetails[6][40];
readFile(listofdetails);
printf("%s \n", listofdetails[1]);
return 0;
}
void readFile(char listofdetails[6][40])
{
int i;
FILE *fp;
fp = fopen("specification.txt", "r");
for(i=0;i<6;i++)
{
fgets(listofdetails[i], 40, fp);
printf("%s \n", listofdetails[i]);
}
fclose(fp);
}
Several options:
First option - you could return listOfDetails directly:
#define NUM_DETAILS 6
#define DETAIL_LENGTH 40
...
char **readFile( void )
{
char **listOfDetails =
malloc( sizeof *listOfDetails * NUM_DETAILS ); // note operand of sizeof
if ( listOfDetails )
{
...
listOfDetails[i] =
malloc( sizeof *listOfDetails[i] * DETAIL_LENGTH ); // note operand of sizeof
...
}
return listOfDetails;
}
int main( void )
{
...
char **details = readFile();
if ( details )
{
...
for ( int i = 0; i < NUM_DETAILS; i++ )
{
free( details[i] );
}
free( details );
}
}
This is probably the straightest path from what you currently have written. Note that in the malloc calls, I use sizeof *listOfDetails and sizeof *listOfDetails[i]. The type of the expression *listOfDetails is char *, so sizeof *listOfDetails is equivalent to sizeof (char *). Similarly, the type of the expression *listOfDetails[i] is char, so sizeof *listOfDetails[i] is equivalent to sizeof (char).
This helps minimize maintenance headaches if you ever decide to change the type of listOfDetails. It's also a little easier to read IMO.
Note that main is now responsible for freeing the memory allocated by readFile; ideally, you'd like to avoid sharing memory management responsibility like this, but sometimes it can't be helped.
Second option - you could pass listOfDetails as an argument to your function:
void readFile( char ***listOfDetails ) // note extra level of indirection
{
...
*listOfDetails = malloc( sizeof **listOfDetails * NUM_DETAILS );
if ( *listOfDetails )
{
...
(*listOfDetails)[i] =
malloc( sizeof *(*listOfDetails)[i] * DETAIL_LENGTH ); // parens are required!
...
}
}
int main( void )
{
char **listOfDetails
...
readFile( &listOfDetails ); // note & operator; I want readFile to modify the value
// stored in listOfDetails, so I must pass a
// pointer to it; since the type of listOfDetails
// is char **, the resulting pointer will have type
// char ***, which is seen in the declaration of
// readFile above.
...
// free as above
}
There isn't a ton of difference between this option and the first, except that it adds an extra level of indirection (which some people will find objectionable). You still have shared memory management responsibility. Note that the parentheses are required in the expression (*listOfDetails)[i]; we don't want to subscript listOfDetails directly, we want to subscript what it points to. Since [] has higher precedence than unary *, we have to explicitly group operators and operands.
Third option - Create separate functions to allocate and free the memory, call both from main:
char **createDetailList( size_t numDetails, size_t detailLength )
{
char **details = malloc( sizeof *details * numDetails );
if ( details )
{
for ( size_t i = 0; i < NUM_DETAILS; i++ )
{
details[i] = malloc( sizeof *details[i] * detailLength );
}
}
return details;
}
void freeDetailList( char **detailList, size_t numDetails )
{
for ( size_t i = 0; i < numDetails; i++ )
free( detailList[i] );
free( detailList );
}
void readFile( char **listOfDetails )
{
...
fread( listOfDetails[i], DETAIL_LENGTH, fp );
...
}
int main( void )
{
char **listOfDetails = createDetailList( NUM_DETAILS, DETAIL_LENGTH );
...
readFile( listOfDetails ); // note no & operator in this case
...
freeDetailList( listOfDetails, NUM_DETAILS );
}
This way, main is responsible for both allocating and deallocating the array. Breaking the memory management code out into separate functions serves two purposes: one, it reduces clutter in main; two, it allows us to re-use this code. Suppose you wanted to read multiple lists of details from multiple files for some reason:
int main ( void )
{
char **list1 = createDetailList( NUM_DETAILS, DETAIL_LENGTH );
char **list2 = createDetailList( NUM_DETAILS, DETAIL_LENGTH );
...
readFile( list1, "file1.txt" ); // Note that we're passing the name of the file
readFile( list2, "file2.txt" ); // as an argument; the definition of readFile
// will need to change accordingly.
...
freeDetailList( list1, NUM_DETAILS );
freeDetailList( list2, NUM_DETAILS );
}
You could also create lists of different sizes (10 records of 80 characters each, say), although you'd need to convey that information to the readFile function somehow (usually as function arguments).
This works because you know ahead of time how big your array needs to be. If you don't know ahead of time how many detail records you need to read or the maximum length of each detail line, then this approach won't work, and you're back to allocating memory as you go in the readFile function.
Note that in this case, I don't expect readFile to modify listOfDetails, so I'm not passing a pointer to it.
Fourth option - if your array is fixed in size and not terribly big (which seems to be the case here), don't bother with dynamic memory management at all. Declare it in main as a regular array and pass it to the function as an argument:
void readFile( char (*listOfDetails)[DETAIL_LENGTH], size_t numDetails ) // or char listOfDetails[][DETAIL_LENGTH]
{
...
for(i=0;i<numDetails;i++)
{
fgets(listofdetails[i], DETAIL_LENGTH, fp);
printf("%s \n", listofdetails[i]);
}
,,,
}
int main( void )
{
char details[NUM_DETAILS][DETAIL_LENGTH];
...
readFile( details, NUM_DETAILS ); // note no & operator; when an array expression
// appears in most contexts, it will be converted
// ("decay") to a pointer to the first element.
// In this case, details will be converted to a
// pointer to an array of char, as you can see in
// the declaration of the readFile function.
// Note that I'm also passing the number of rows
// explicitly.
...
}
Advantages: no memory management hijinks, no multiple dereferences, etc. The main drawback is that it can only work with arrays where the number of columns is DETAIL_LENGTH; you can't use it to read a 6x50 array, for example. If you're only using the one array, this isn't a problem.
Pointers don't know whether they're pointing to a single object or a sequence of objects in an array, so you usually need to specify the number of rows as a separate parameter.
You could use the NUM_DETAILS constant, but ideally functions should communicate exclusively through parameter lists and return values; they should not share state indirectly by relying on global variables or magic constants. This "decouples" the function from the larger program, making it easier to reuse. For example, I'd define your readFile function as
void readFile( char **listOfDetails,
size_t numDetails,
size_t detailLength,
const char *filename )
{
FILE *fp = fopen( filename, "r" );
if ( fp )
{
for( size_t i = 0; i < numDetails; i++ )
{
fread( listOfDetails[i], detailLength, fp ); // no error checking for brevity
}
}
fclose( fp );
}
This function doesn't rely on any shared information; it can easily be re-used by any other program, or used to read multiple lists of different sizes in the same program.
Note that in all four cases I replace the literals 6 and 40 with symbolic constants. This serves two purposes: first, the symbol names are more meaningful than the raw numbers; second, if you decide you want to read 7 detail records, or that the records now need to be 80 characters long, all you need to do is change the definition of the constants - you don't have to dig through the code and replace every instance of 6 or 40.
You could modify your function to
char ** readFile()
and then at the end of the function definition add
return listofdetails;
As pointed out below, this works provided that the size of listofdetails is fixed and therefore you can write a counterpart function that frees the allocated memory.
