Help with type returned

Discussion in 'C Programming' started by leo2100@gmail.com, Dec 9, 2005.

  1. Guest

    Hi, I need help with this program. The program is supposed to take a
    text file and identify the words in it, then it should print them and
    count how many times a word is repeated. At first main called the
    function wordcount, and then the function did everything including
    printing out the results. That worked. Now I want to make the function
    return an array of pointers to struct palabra so the calling function
    can manage the data as it pleases. What I`m having trouble with, is to
    make the function return a pointer to the array of pointers. I`m using
    Dev-C++ compiler and the error says "return from incompatible pointer
    type" in the line "return pal;" at the end of the function. I tried a
    hundred things but nothing seems to work, if it`s not the same error
    it`s "conversion to non-scalar type requested" on the same line. Any
    help, as well as tips to make the programa better is welcome and
    appreciated.

    CODE:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #define NUL '\0'

    struct palabra
    {
    char letras[20];
    short veces;
    };

    typedef struct palabra spal;

    spal *wordcount(FILE *archivo)
    {
    char chr, str[20];
    struct palabra *pal[1000];
    signed short i=0, j=0, k=0, p=0;

    for (j = 0; j<20; ++j) str[j] = (char) NUL;

    for (i = 0; ; ++i)
    {
    chr = fgetc(archivo);
    switch (chr)
    {
    case '.':
    str = (char) NUL;
    i = -1;
    break;

    case ',':
    str = (char) NUL;
    i = -1;
    break;

    case ':':
    str = (char) NUL;
    i = -1;
    break;

    case ' ':
    str = (char) NUL;
    i = -1;
    break;

    case ';':
    str = (char) NUL;
    i = -1;
    break;

    case '!':
    str = (char) NUL;
    i = -1;
    break;

    case '?':
    str = (char) NUL;
    i = -1;
    break;

    case (char) NUL:
    str = (char) NUL;
    i = -1;
    break;

    case EOF:
    str = (char) NUL;
    i = -1;
    break;

    default:
    str = chr;
    break;
    }
    if (i == -1)
    {
    if (str[0] != (char) NUL)
    {
    if (p==0)
    {
    pal[p] = malloc(sizeof(spal));
    pal[p]->veces = 1;
    for (j = 0; j<20; ++j) pal[p]->letras[j] =
    (char) NUL;
    for (j = 0; str[j] != (char) NUL; ++j)
    {
    pal[p]->letras[j] = str[j];
    }
    if (p == 1000) printf("El archivo contiene mas
    de mil palabras !!!");
    ++p;
    }
    else
    {
    k=0;
    for (j = 0; j<p; ++j)
    {
    if ((strcmp(str, pal[j]->letras)) == 0)
    {
    ++pal[j]->veces;
    ++k;
    }
    }
    if (k == 0)
    {

    pal[p] = malloc(sizeof(spal));
    pal[p]->veces = 1;
    for (j = 0; j<20; ++j) pal[p]->letras[j] =
    (char) NUL;
    for (j = 0; str[j] != (char) NUL; ++j)
    {
    pal[p]->letras[j] = str[j];
    }
    ++p;
    }
    }
    for (j = 0; j<= 20; ++j) str[j] = (char) NUL;
    }
    }
    if (chr == EOF) break;
    }

    if (p==0)
    {
    printf("El archivo no contiene ninguna palabra\n");
    return NULL;
    }
    else
    {
    return pal;
    /* for (i = 0; i<p; ++i)
    {
    printf("%s, Veces: %d\n", pal->letras, pal->veces);
    }*/
    }
    }

    int main(int argc, char *argv[])
    {
    FILE *texto;
    struct palabra *pal[1000];
    short i;

    /* if ((texto = fopen("prueba.txt", "w+")) == NULL)
    {
    printf("Error tratando de abrir el archivo\n");
    system("PAUSE");
    exit(1);
    }

    fputs ("Hello World", texto);

    if ((fclose(texto)) != 0) printf("No se pudo cerrar correctamente el
    archivo\n");*/

    if ((texto = fopen("prueba.txt", "r")) == NULL)
    {
    printf("Error tratando de abrir el archivo\n");
    system("PAUSE");
    exit(1);
    }

    if ((pal = wordcount(texto)) == NULL)
    {
    printf("La operacion fallo\n");
    system("PAUSE");
    exit(1);
    }

    for (i = 0; pal->letras[0] != NUL; ++i)
    {
    printf("%s, Veces: %d\n", pal->letras, pal->veces);
    }

    if ((fclose(texto)) != NULL) printf("No se pudo cerrar correctamente
    el archivo\n");

    system("PAUSE");
    return 0;
    }
     
    , Dec 9, 2005
    #1
    1. Advertising

  2. In article <>,
    <> wrote:
    >Hi, I need help with this program.


    >Now I want to make the function
    >return an array of pointers to struct palabra so the calling function
    >can manage the data as it pleases. What I`m having trouble with, is to
    >make the function return a pointer to the array of pointers. I`m using
    >Dev-C++ compiler and the error says "return from incompatible pointer
    >type" in the line "return pal;" at the end of the function.


    >struct palabra
    >{
    > char letras[20];
    > short veces;
    >};


    Okay, that's the structure itself.


    >typedef struct palabra spal;


    That's an alias for the structure. Incidently, you could have done that
    in one step, via

    typedef struct palabra { char letras[20]; short veces; } spal;


    >spal *wordcount(FILE *archivo)


    Since spal is an alias for the struct, spal * means a pointer to one
    structure, technically, but because of pointer arithmetic, spal *
    also stand in for a pointer to the first element of an array of
    the structures.

    >{
    > char chr, str[20];
    > struct palabra *pal[1000];


    struct palabra has been aliased with spal, so this line could have
    also been written as

    spal *pal[1000];

    That's an array of 1000 pointers to structures.

    > if (p==0)
    > {
    > pal[p] = malloc(sizeof(spal));


    Okay, that's consistant, malloc returns a pointer to a block
    of the appropriate size, so effectively the right hand side of that
    line has type *spal; you are then storing that pointer into
    an array element that is properly typed to hold such pointers.

    > pal[p]->veces = 1;


    That's consistant too.

    > pal[p]->letras[j] = str[j];


    And that is consistant as well.

    > pal[p] = malloc(sizeof(spal));
    > pal[p]->veces = 1;


    Those are fine as well.

    > pal[p]->letras[j] = str[j];


    The types there are fine too.

    > else
    > {
    > return pal;


    But look at that. pal is an automatic variable which is an array,
    each of whose elements is a pointer to a structure. In that
    context (and -most- other contexts), a reference to the array name
    will automatically be converted to the first element of the
    array. Thus, pal will, in that context, devolve into a pointer
    to what the first element is, and the first element is a pointer
    to a structure, so pal is a pointer to a pointer to a structure.
    But your return type is pointer to structure, not pointer to pointer
    to structure, so the compiler is complaining.

    Even if you got the type right, you would have problems, because you
    would be trying to return a pointer to an automatic variable to
    outside the scope that the variable exists in.


    >int main(int argc, char *argv[])
    >{


    > struct palabra *pal[1000];


    This is the same declaration as for pal in the subroutine, so
    this gets you a variable named pal which is a real array
    (that is, memory allocated for it) of 1000 elements long,
    each of which is a pointer to a structure.

    > if ((pal = wordcount(texto)) == NULL)


    But there you try to assign a value to pal . You can't do that,
    because pal is not a pointer: it is an array. Effectively,
    the name of an array is something that is a constant value as
    far as that scope is concerned.

    > for (i = 0; pal->letras[0] != NUL; ++i)


    Looking at that, it looks like you really do want pal to
    come out as a something that could dereferrenced to be
    a pointer to a structure, rather than wanting pal to be
    an array of structures. That is consistant with everything
    in your subroutine except the return value you declared for
    the subroutine.


    I would suggest to you that unless you have reason otherwise,
    that the easiest way to fix your program would be to declare
    pal in your main program, and then pass it in as a parameter
    to your subroutine, which would write into it, with the
    subroutine not trying to return a pointer to anything.


    A few other remarks:

    - If your program happens to fill in -exactly- 1000 words,
    pal[0] through pal[999], then there will be no entries for
    which pal[p]->letras[0] is NUL, so you would run off the
    end of the array.

    - pal[p] does not get anything written into it unless there
    is a word to go there, but if there is a word to go there
    then pal[p]->letras[0] is not going to be NUL. When
    in the main program after you have examined the last entry
    you created, you are going to increment p and try to look
    at pal[p]->letras but pal[p] is not going to have been assigned
    any value. You will probably crash at that point, if you are lucky.

    - Your cases can all be compacted into just two cases:

    case ':': case ';': case '!': (and so on)
    str = (char) NUL;
    i = -1;
    break;

    default:
    str = chr;
    break;


    You can have many different "case" prefixes for the same block of code.

    - I would suggest that you consider replacing your switch() with a
    test such as

    if ( isalpha(chr) ) { str = (char) NUL; break }
    str = chr;

    You will have to decide what you want to do with numbers and characters
    such as @ . I would suggest to you that you probably do not want
    to consider characters such () to be part of a word (otherwise when
    you analyzed this sentance, you would end up with a word "(otherwise"
    complete with the "(" and that would be a different word than
    "otherwise" (which would be stored complete with quotation marks)
    and your matches would be different than they otherwise would...)
    --
    "law -- it's a commodity"
    -- Andrew Ryan (The Globe and Mail, 2005/11/26)
     
    Walter Roberson, Dec 9, 2005
    #2
    1. Advertising

  3. Guest

    Walter Roberson wrote:
    > In article <>,
    > <> wrote:
    > >Hi, I need help with this program.

    >
    > >Now I want to make the function
    > >return an array of pointers to struct palabra so the calling function
    > >can manage the data as it pleases. What I`m having trouble with, is to
    > >make the function return a pointer to the array of pointers. I`m using
    > >Dev-C++ compiler and the error says "return from incompatible pointer
    > >type" in the line "return pal;" at the end of the function.

    >
    > >struct palabra
    > >{
    > > char letras[20];
    > > short veces;
    > >};

    >
    > Okay, that's the structure itself.
    >
    >
    > >typedef struct palabra spal;

    >
    > That's an alias for the structure. Incidently, you could have done that
    > in one step, via
    >
    > typedef struct palabra { char letras[20]; short veces; } spal;
    >
    >
    > >spal *wordcount(FILE *archivo)

    >
    > Since spal is an alias for the struct, spal * means a pointer to one
    > structure, technically, but because of pointer arithmetic, spal *
    > also stand in for a pointer to the first element of an array of
    > the structures.
    >
    > >{
    > > char chr, str[20];
    > > struct palabra *pal[1000];

    >
    > struct palabra has been aliased with spal, so this line could have
    > also been written as
    >
    > spal *pal[1000];
    >
    > That's an array of 1000 pointers to structures.
    >
    > > if (p==0)
    > > {
    > > pal[p] = malloc(sizeof(spal));

    >
    > Okay, that's consistant, malloc returns a pointer to a block
    > of the appropriate size, so effectively the right hand side of that
    > line has type *spal; you are then storing that pointer into
    > an array element that is properly typed to hold such pointers.
    >
    > > pal[p]->veces = 1;

    >
    > That's consistant too.
    >
    > > pal[p]->letras[j] = str[j];

    >
    > And that is consistant as well.
    >
    > > pal[p] = malloc(sizeof(spal));
    > > pal[p]->veces = 1;

    >
    > Those are fine as well.
    >
    > > pal[p]->letras[j] = str[j];

    >
    > The types there are fine too.
    >
    > > else
    > > {
    > > return pal;

    >
    > But look at that. pal is an automatic variable which is an array,
    > each of whose elements is a pointer to a structure. In that
    > context (and -most- other contexts), a reference to the array name
    > will automatically be converted to the first element of the
    > array. Thus, pal will, in that context, devolve into a pointer
    > to what the first element is, and the first element is a pointer
    > to a structure, so pal is a pointer to a pointer to a structure.
    > But your return type is pointer to structure, not pointer to pointer
    > to structure, so the compiler is complaining.
    >
    > Even if you got the type right, you would have problems, because you
    > would be trying to return a pointer to an automatic variable to
    > outside the scope that the variable exists in.
    >
    >
    > >int main(int argc, char *argv[])
    > >{

    >
    > > struct palabra *pal[1000];

    >
    > This is the same declaration as for pal in the subroutine, so
    > this gets you a variable named pal which is a real array
    > (that is, memory allocated for it) of 1000 elements long,
    > each of which is a pointer to a structure.
    >
    > > if ((pal = wordcount(texto)) == NULL)

    >
    > But there you try to assign a value to pal . You can't do that,
    > because pal is not a pointer: it is an array. Effectively,
    > the name of an array is something that is a constant value as
    > far as that scope is concerned.
    >
    > > for (i = 0; pal->letras[0] != NUL; ++i)

    >
    > Looking at that, it looks like you really do want pal to
    > come out as a something that could dereferrenced to be
    > a pointer to a structure, rather than wanting pal to be
    > an array of structures. That is consistant with everything
    > in your subroutine except the return value you declared for
    > the subroutine.
    >
    >
    > I would suggest to you that unless you have reason otherwise,
    > that the easiest way to fix your program would be to declare
    > pal in your main program, and then pass it in as a parameter
    > to your subroutine, which would write into it, with the
    > subroutine not trying to return a pointer to anything.
    >
    >
    > A few other remarks:
    >
    > - If your program happens to fill in -exactly- 1000 words,
    > pal[0] through pal[999], then there will be no entries for
    > which pal[p]->letras[0] is NUL, so you would run off the
    > end of the array.
    >
    > - pal[p] does not get anything written into it unless there
    > is a word to go there, but if there is a word to go there
    > then pal[p]->letras[0] is not going to be NUL. When
    > in the main program after you have examined the last entry
    > you created, you are going to increment p and try to look
    > at pal[p]->letras but pal[p] is not going to have been assigned
    > any value. You will probably crash at that point, if you are lucky.
    >
    > - Your cases can all be compacted into just two cases:
    >
    > case ':': case ';': case '!': (and so on)
    > str = (char) NUL;
    > i = -1;
    > break;
    >
    > default:
    > str = chr;
    > break;
    >
    >
    > You can have many different "case" prefixes for the same block of code.
    >
    > - I would suggest that you consider replacing your switch() with a
    > test such as
    >
    > if ( isalpha(chr) ) { str = (char) NUL; break }
    > str = chr;
    >
    > You will have to decide what you want to do with numbers and characters
    > such as @ . I would suggest to you that you probably do not want
    > to consider characters such () to be part of a word (otherwise when
    > you analyzed this sentance, you would end up with a word "(otherwise"
    > complete with the "(" and that would be a different word than
    > "otherwise" (which would be stored complete with quotation marks)
    > and your matches would be different than they otherwise would...)
    > --
    > "law -- it's a commodity"
    > -- Andrew Ryan (The Globe and Mail, 2005/11/26)


    Hi, first of all, thanks for the response.

    I have addressed every one of your suggestions. The result, or at least
    the partial result is this new program:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #define NUL '\0'

    typedef struct palabra
    {
    char letras[20];
    short veces;
    } spal;

    spal **wordcount(FILE *archivo, spal **pal)
    {
    char chr, str[20];
    signed short i=0, j=0, k=0, p=0;

    for (j = 0; j<20; ++j) str[j] = (char) NUL;

    for (i = 0; ; ++i)
    {
    chr = fgetc(archivo);

    if ( !isalpha(chr) )
    {
    str = (char) NUL;
    i = -1;
    }
    else str = chr;

    if (i == -1)
    {
    if (str[0] != (char) NUL)
    {
    ++p;
    for (j = 0; j<= 20; ++j) str[j] = (char) NUL;
    }
    }
    if (chr == EOF) break;
    }

    if (p==0)
    {
    printf("El archivo no contiene ninguna palabra\n");
    return NUL;
    }

    rewind (archivo);

    if ((pal = malloc ((sizeof(spal **)) * (p+1))) == NULL) return
    NUL;

    p=0;

    for (i = 0; i<20 ; ++i)
    {
    chr = fgetc(archivo);

    if ( !isalpha(chr) )
    {
    str = (char) NUL;
    i = -1;
    }
    else str = chr;

    if (i == -1)
    {
    if (str[0] != (char) NUL)
    {
    if (p == 0)
    {
    if ((pal[0] = malloc(sizeof(spal))) == NULL)
    return NUL;
    pal[0]->veces = 1;
    for (j = 0; j<20; ++j) pal[0]->letras[j] =
    (char) NUL;

    for (j = 0; str[j] != (char) NUL; ++j)
    {
    pal[0]->letras[j] = str[j];
    }
    ++p;
    }
    else
    {
    k=0;
    for (j = 0; j<p; ++j)
    {
    if ((strcmp(str, pal[j]->letras)) == 0)
    {
    ++pal[j]->veces;
    ++k;
    }
    }
    if (k == 0)
    {
    if ((pal[p] = malloc(sizeof(spal))) == NULL)
    return NUL;
    pal[p]->veces = 1;
    for (j = 0; j<20; ++j) pal[p]->letras[j] =
    (char) NUL;

    for (j = 0; str[j] != (char) NUL; ++j)
    {
    pal[p]->letras[j] = str[j];
    }
    ++p;
    }

    }
    }
    for (j = 0; j<= 20; ++j) str[j] = (char) NUL;
    }
    if (chr == EOF) break;
    }
    if (i >= 20) return NUL;

    if ((pal[p] = malloc(sizeof(spal))) == NULL) return NUL;
    for (j = 0; j<20; ++j) pal[p]->letras[j] = (char) NUL;

    /* for (i = 0; pal->letras[0] != NUL; ++i)
    {
    printf("%s, Veces: %d\n", pal->letras,
    pal->veces);
    }*/

    return pal;
    }

    int main(int argc, char *argv[])
    {
    FILE *texto;
    spal **pal;
    short i;

    /* if ((texto = fopen("prueba.txt", "w+")) == NULL)
    {
    printf("Error tratando de abrir el archivo\n");
    system("PAUSE");
    exit(1);
    }

    fputs ("Hello World", texto);

    if ((fclose(texto)) != 0) printf("No se pudo cerrar correctamente el
    archivo\n");*/

    if ((texto = fopen("prueba.txt", "r")) == NULL)
    {
    printf("Error tratando de abrir el archivo\n");
    system("PAUSE");
    exit(1);
    }

    if ((pal = wordcount(texto, pal)) == NUL)
    {
    printf("La operacion fallo\n");
    system("PAUSE");
    exit(1);
    }

    for (i = 0; pal->letras[0] != NUL; ++i)
    {
    printf("%-20s, Veces: %d, i=%d\n", pal->letras,
    pal->veces, i);
    }

    if ((fclose(texto)) != NULL) printf("No se pudo cerrar correctamente
    el archivo\n");

    system("PAUSE");
    return 0;
    }

    I have rearranged everything so that:
    -First the program counts the number of words in the text file
    (repeated ones are included in the count). This way I can dynamically
    create an array with the number of words in the text file instead of
    just predefining a maximum number of words.
    -With that number it then allocates space for p number of pointers to
    spal and then assigns the start of this "array" to pal. The space of
    the repeated words pointers is wasted, but I preferred to do that than
    to check for repeated words during the count.
    -Then it proceeds to do the same thing as before, search for new words,
    allocate new spals and store the words as strings in them.
    -Finally it sets the last string to be always NUL filled and returns
    the pointer to the array of pointers to the calling function.

    With the last program you made me realize that returning a pointer to
    the start of an array that has been created in the called function is
    pointless, because the array gets destroyed when the function returns
    (I think that's what you meant by automatic variable). So I decided to
    use malloc to create the array, I figured that the array would exist as
    long as it is not freed or the program exits.

    I tried to do what you said about passing a pointer to the function and
    then let the function modify it so that it doesn´t need to return it,
    but I have failed doing so. I don´t know how the type should be
    defined so that happens, all I get is the compiler complaining.

    Thanks for letting me know about the function isalpha, I`m new to C,
    and I don´t know many functions. The function worked perfectly and it
    excludes @ and () already.

    Somehow I`m having trouble returning a NULL pointer, I read a FAQ about
    it, and concluded that if I use NULL with pointers there should be no
    problems, but the compiler proves otherwise. I used NUL instead.

    Finally I would like to now if the program is fair enough, i.e. I want
    to know if it could be optimized or if I did something that didn´t
    need doing.
     
    , Dec 10, 2005
    #3
    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. Lasse Edsvik

    What "type" is returned?

    Lasse Edsvik, Sep 24, 2004, in forum: ASP .Net
    Replies:
    2
    Views:
    425
    Kevin Spencer
    Sep 24, 2004
  2. Johannes
    Replies:
    2
    Views:
    1,244
    Kevin Spencer
    Mar 15, 2006
  3. Wolfgang
    Replies:
    3
    Views:
    2,358
    Wolfgang
    May 7, 2004
  4. Giovanni Gherdovich
    Replies:
    3
    Views:
    1,207
    Giovanni Gherdovich
    Aug 28, 2008
  5. candide
    Replies:
    2
    Views:
    261
    Ian Kelly
    Dec 27, 2011
Loading...

Share This Page