Newbee needs once more again help with passing arrays out of a function

Discussion in 'C Programming' started by Christian Maier, Feb 17, 2007.

  1. Hi

    After surfing a while I have still trouble with this array thing. I
    have the following function and recive a Segmentation fault, how must
    I code this right??

    Christian Maier

    void fetchIds(llong *ids, char **names)
    llong rC = 0;
    llong i = 0;
    char resultSet1[21]; //max. Length of a long long+1
    char resultSet2[129];

    rC = PQntuples( get_result() ) ;


    printf("Rows: %d\n", rC);
    while (fetch(resultSet1, resultSet2) != END_OF_TUPLES) //
    loop through all rows returned
    //This works fine
    printf("> %s, %s <\n", resultSet1, resultSet2);

    // ---> This is segmentation fault <--- ???
    //names = resultSet2;


    int argc, char *argv[])
    llong *ids;
    char **mNames;

    doquery("select provider_id, caption from fi.provider");
    fetchIds(ids, mNames);

    return 0;
    Christian Maier, Feb 17, 2007
    1. Advertisements

  2. Christian Maier

    santosh Guest

    llong is not defined in standard C. I'm assuming it's an alias for
    long long. If not then you need to show us the relevant declaration.
    I don't see how you conclude that. long long is atleast 64 bits. But
    the string representation of LLONG_MAX needn't be twenty characters.
    You don't show us the definitions for PQntuples, ids and names.
    Hopefully they're all correct.
    The printf format specifier for long long is %lld or it's variants
    like %llx etc.
    Hopefully both resultSet1 and resultSet2 are nul terminated.

    You don't show us your definitions for names and ids, so we can't
    really help you. Based on it's usage a few lines above, names seems to
    be a two-dimensional array. In the above code you're using it with a
    single index.

    It also looks as if ids needs to be a two-D array.
    Please post a minimal compilable subset of your code that exhibits the
    error. Please cut and paste, don't retype. From what you've posted,
    it's nearly impossible to tell what's causing the fault.
    santosh, Feb 17, 2007
    1. Advertisements

  3. Christian Maier

    Guest Guest

    Generally speaking, it's best to copy and paste a compilable program
    into your message, to avoid problems like this. This is a syntax
    error, of course, because int main( got left out for some reason.
    "ids" and "mNames" are pointers. They can contain the starting address
    of some region in memory, but they can only reliably do that if you
    give them the starting address of some region in memory. Look up the
    malloc function to fix this.

    (I haven't looked too closely to see if the rest of your program
    should work properly.)
    Guest, Feb 17, 2007

  4. Hi,

    You haven't given too much information, so I can only guess at what
    could be happening. But a couple of things stick out. Like Harald
    pointed out, the pointers ids, mNames need to be initialized. I'm
    assuming that you have some code to take care of that, which you're
    not showing us. The other thing that looks dangerous to me is
    "names = resultSet2;".

    resultSet2 is a local variable. As a rule of thumb, local variables
    are stored on the stack, which, for the purposes of this discussion,
    means that the memory allocated for a local variable is deallocated
    (note that this is an over-simplification) once the function "ends".
    When you say names = resultSet2, you're copying the address of
    resultSet2 (an address that is only going to be valid while you're
    "inside" fetchIds) into names. I am guessing that your intention is
    to actually copy the data, not the address, in which case, you should
    probably use something like "strncpy(names, resultSet2, MAX_LEN);".
    Of course, names will have to be "properly" allocated memory.

    If this doesn't solve your problem, please post a complete, yet brief,
    snippet, isolating the problem and we can try to see what really going
    Pramod Subramanyan, Feb 18, 2007
  5. There are a lot of things no clear from the code you posted, e.g.
    what exactly the functions you use do, what their return values
    are, what llong is etc. So in trying to write something useful
    I will have to make a lot of assumptions that may not be true.
    Please keep that in mind.

    I will go through the code first and make a few comments and then
    try to give you a version that may be somewhat nearer to what you
    are going to need.
    Here are already several problems. First of all, it looks as
    if you want to pass back an array of integers and an array of
    strings to the calling function (or pointers to these arrays
    to be precise) where the memory for the arrays will get allo-
    cated within this function. And in this case you need the value
    of e.g. 'ids' to be set and have the new value passed back to
    the caller. This won't work when this function just gets an
    llong pointer - arguments get passed by value in C, so what-
    ever changes you do to 'ids' in this function won't be visible
    in the caller. If you want to change 'ids' in a way that can be
    seen by the caller you need to pass a pointer to that pointer
    and treat it accordingly in this function. The same holds for
    'names'. The second problem is that you seem to determine the
    number of elements of these arrays in this function but the
    caller isn't going to know what that is, and you won't be able
    to use those arrays in the caller without that knowledge. You
    must return that length either via another argument or as the
    return value of the function.
    This seems to indicate that llong is a 64-bit long long, at
    least 21 chars are exactly the right amount for that case
    (2^63 is about 9.2e18, so 19 chars are needed for that plus
    one for a minus sign and another one for the trailing '\0').
    I've got to believe you that 129 chars will always be enough
    Looks like this function returns the number of calls of fetch()
    you can do.
    Now that won't make it past your compiler as you probably have
    seen;-) It looks a lot as if you would like to allocate memory
    for an array of llongs and another one for some kind of 2-dimen-
    sional array (I write "some kind" since a 2-dimensional array is
    *not* of type pointer to pointer, but that's what 'names' is).
    Since 'rC' seems to be a long long you will need '%lld' here
    as the conversion specifier.
    Can you be 100% sure that this condition is true for not more than
    'rC' times? And I hope that we can assume that both 'resultSet1'
    and 'resultSet2' are properly '\0'-terminated strings.

    You will need to use strcopy() or something similar here. A simple
    assignment just copies the address of 'resultSet2' and not the
    string in 'resultSet2'. Moreover, once you left the function, that
    address has gone out of scope and can't be used anymore.

    Ok, as promised, here's a version that hopefully will be a bit more
    like what you are going to need (of course only if the assumptions
    I had to make aren't too far off the mark):

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

    #define llong long long

    llong fetchIds( llong **ids, char ***names )
    llong rC = 0;
    llong i;
    char resultSet1[ 21 ];
    char resultSet2[ 129 ];
    llong *l_ids; /* temporary variables that make */
    char ** l_names; /* things a bit easier to read */

    rC = PQntuples( get_result( ) ) ;

    /* Get memory for an array of 'rC' llongs - lets hope that the
    number of bytes will always be in the range of size_t, one
    could ceck for that to be 100% safe. */

    if ( ( l_ids = malloc( rC * sizeof *l_ids ) ) == NULL )
    fprintf( stderr, "Not enough memory, bailing out.\n" );
    exit( EXIT_FAILURE );

    /* Get memory for an array of 'rC' char pointers */

    if ( ( l_names = malloc( rC * sizeof *l_names ) ) == NULL )
    fprintf( stderr, "Not enough memory, bailing out.\n" );
    exit( EXIT_FAILURE );

    printf( "Rows: %lld\n", rC );

    /* Loop until either fetch() has been called 'rC' times or its
    return value indicates that we're done */

    for ( i = 0;
    i < rC && fetch( resultSet1, resultSet2 ) != END_OF_TUPLES;
    i++ )
    /* First we need memory for the string we're going to copy.
    The address gets stored in the ith element of the array
    of char pointers we allocated before. And instead of
    129 (as in the origional code) the length of the string
    we're going to copy is used in order to avoid allocating
    more memory than necessary. */

    l_names[ i ] = malloc( ( strlen( resultSet2 ) + 1 ) *
    sizeof **l_names );
    if ( l_names[ i ] == NULL )
    fprintf( stderr, "Not enough memory, bailing out.\n" );
    exit( EXIT_FAILURE );

    strcpy( l_names[ i ], resultSet2 );

    /* Probably one should do some error checking for the next
    line - strtoll() could fail for some reason like the
    string to be converted not containing a number */

    l_ids[ i ] = strtoll( resultSet1, NULL, 0 );

    /* Now that we're done the addresses of the newly allocated
    arrays get stored in the argument pointers. */

    *ids = l_ids;
    *names = l_names;

    /* Return how many times we called fetch() successfully and
    thus how many elements in both the arrays have been set. */

    return i;

    int main( void )
    llong rows;
    llong *ids;
    char **mNames;
    llong i;

    /* Call fetchIds() with the addresses of both the pointers so
    that the values of the pointers can be changed from within
    that function. */

    rows = fetchIds( &ids, &mNames );

    /* Print out what we just got */

    for ( i = 0; i < rows; i++ )
    printf( "%lld => %s\n", ids[ i ], mNames[ i ] );

    /* Sometime later you will have to get rid of the memory that
    was allocated in fetchIds(). That can be done like this: */

    for ( i = 0; i < rows; i++ )
    free( mNames[ i ] );
    free( mNames );
    free( ids );

    return EXIT_SUCCESS;

    Please note again: 'mNames' is not a 2-dimensional array like one
    that you would define like this

    char real_2d_array[ 123 ][ 129 ];

    Instead 'mNames' is an array of char pointers and each of these char
    pointers stored in 'mNames' points in turn to an array of chars (or,
    to be precise, the first element of the char array). See also the
    C-FAQ (section 6.16) about this

    Regards, Jens
    Jens Thoms Toerring, Feb 18, 2007
    1. Advertisements

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments (here). After that, you can post your question and our members will help you out.