Walter Roberson wrote:
In article <11************ **********@g44g 2000cwa.googleg roups.com>,
<le*****@gmail. com> 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(s pal));
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(s pal));
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[i]->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[i] = (char) NUL;
i = -1;
break;
default:
str[i] = 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[i] = (char) NUL; break }
str[i] = 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(FIL E *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[i] = (char) NUL;
i = -1;
}
else str[i] = 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[i] = (char) NUL;
i = -1;
}
else str[i] = chr;
if (i == -1)
{
if (str[0] != (char) NUL)
{
if (p == 0)
{
if ((pal[0] = malloc(sizeof(s pal))) == 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(s pal))) == 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(s pal))) == NULL) return NUL;
for (j = 0; j<20; ++j) pal[p]->letras[j] = (char) NUL;
/* for (i = 0; pal[i]->letras[0] != NUL; ++i)
{
printf("%s, Veces: %d\n", pal[i]->letras,
pal[i]->veces);
}*/
return pal;
}
int main(int argc, char *argv[])
{
FILE *texto;
spal **pal;
short i;
/* if ((texto = fopen("prueba.t xt", "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.t xt", "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[i]->letras[0] != NUL; ++i)
{
printf("%-20s, Veces: %d, i=%d\n", pal[i]->letras,
pal[i]->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.
