Pointers again...

I

ifmusic

i have this little .c, it's in spanish but you'll get it anyway

#include <stdio.h>
typedef struct {
char cadena[20];
int enterito;
} registro;

//this puts a new element to the list
int agregarElemento(registro **lista, registro *aux, int *lMax);
int main(void) {
registro **lista;
registro *aux;
int lMax;
int i,a,j;
char buf[15];
//creo una lista de 1 posicion
// i put a single element first
lista = (registro**) malloc (sizeof(registro*));
aux = (registro*) malloc(sizeof(registro));
aux->enterito = 0;
strcpy(aux->cadena, "Cadena_0\0");
lista[0]=aux;

printf("Cadena: %s y el numero es: %d\n",lista[0]->cadena,
lista[0]->enterito);

lMax = 1;

// i'll put N elements more.
//agrego N elementos MAS hasta 0.

printf("Ingresa un numero:\n");
scanf("%s",buf);
j = atoi(buf);

while (j)
{

for (i=0; i< j ; i++)
{
//i fill a record.
sprintf(buf,"Cadena_%d\0",lMax);
aux = (registro*) malloc(sizeof(registro));
aux->enterito = lMax;
strcpy(aux->cadena,buf );
// This is the line that's bugging me ////////////////////////////
lista = (registro**) realloc ( lista, sizeof(registro*) * ((lMax)+1)
);
agregarElemento(lista,aux,&lMax);

}

printf("Ingresa un numero:\n");
scanf("%s",buf);
j = atoi(buf);

}

for (i=0; i< lMax; i++) printf("Cadena: %s y el numero es:
%d\n",lista->cadena, lista->enterito);

return 1;
}
int agregarElemento(registro **lista, registro *aux, int *lMax)
{
lista[*lMax]=aux;
(*lMax)++;
return *lMax;
}


Ok, this works fine, i mean, i can add and print as many items as i
want BUT!
if i want to take the Realloc of the Index (pointer to pointer var.) to
a function like "agregarElemento" <add element >, eventualy it SEGS
FAULT . So there must be something i dont know about pointers, what
happens when you pass it to a function outside ? Because when i do
....... BUM!
If you want i can post a version of the software that fails. I guess
the question is:

How can i make a safe <add_a_element> function?
 
M

Michael Mair

i have this little .c, it's in spanish but you'll get it anyway

#include <stdio.h>
typedef struct {
char cadena[20];

Magic number: Use a symbolic constant instead
#define CADENA_LENGTH 19
....
char cadena[CADENA_LENGTH+1];
where the +1 stems from the string terminator and
CADENA_LENGTH is the usable number of bits.
int enterito;
} registro;

//this puts a new element to the list
int agregarElemento(registro **lista, registro *aux, int *lMax);
int main(void) {
registro **lista;
registro *aux;
int lMax;
int i,a,j;
char buf[15];
//creo una lista de 1 posicion
// i put a single element first
lista = (registro**) malloc (sizeof(registro*));
aux = (registro*) malloc(sizeof(registro));

The cast is unnecessary in C; in addition, this breaks
easily if you change the type. The best practice form around
here is
lista = malloc(sizeof *lista);
aux = malloc(sizeof *aux);
Of course you can put parentheses around *lista and *aux if
that makes you feel more comfortable.
aux->enterito = 0;

Serious error: You did not check whether aux and lista are
!= NULL. You may already have invoked undefined behaviour.
Everything is possible from now on.
strcpy(aux->cadena, "Cadena_0\0");
lista[0]=aux;

printf("Cadena: %s y el numero es: %d\n",lista[0]->cadena,
lista[0]->enterito);

lMax = 1;

// i'll put N elements more.
//agrego N elementos MAS hasta 0.

printf("Ingresa un numero:\n");
scanf("%s",buf);
j = atoi(buf);

Bad choice for input. Consider fgets() and strtol() instead -- they
give you much better information if it comes to finding errors.
while (j)
{

for (i=0; i< j ; i++)
{
//i fill a record.
sprintf(buf,"Cadena_%d\0",lMax);

If you have snprintf(), use it instead.
Otherwise, restrict lMax in a way that makes sure that
Cadena_%d cannot exceed CADENA_LENGTH.
aux = (registro*) malloc(sizeof(registro));
aux->enterito = lMax;
strcpy(aux->cadena,buf );
// This is the line that's bugging me ////////////////////////////
lista = (registro**) realloc ( lista, sizeof(registro*) * ((lMax)+1)
);

You need a temporary variable:
registro **tmp;

....
tmp = realloc (lista, (lMax + 1) * sizeof *tmp);
if (tmp == NULL) {
/* Your error handling here; either break out of the loop
** or exit afterwards. */
}
lista = tmp;
By the way: Why do you not store all registro objects in one array
instead of storing pointers to them in an array?
If you really need to do it this way, then at least do not realloc()
unnecessarily much:
while (j) {
int i;
registro **tmp = realloc (lista, (lMax + j) * sizeof *tmp);
if (tmp == NULL) {
fprintf(stderr, "Memory allocation failed for list extension "
"from %d to %d\n", lMax, lMax + j);
break;
}
lista = tmp;
for (i = 0; i < j; i++) {
lista[lMax + i] = NULL;
}
for (i = 0; i < j; i++) {
if ((aux = malloc(sizeof *aux)) == NULL) {
fprintf(stderr, "Memory allocation failed for list member "
"at %d\n", lMax+i);
break;
}
lista[lMax] = aux;
/* Init *aux */
++lMax;
}
if (aux == NULL)
break;
....
agregarElemento(lista,aux,&lMax);

}
Ok, this works fine, i mean, i can add and print as many items as i
want BUT!
if i want to take the Realloc of the Index (pointer to pointer var.) to
a function like "agregarElemento" <add element >, eventualy it SEGS
FAULT . So there must be something i dont know about pointers, what
happens when you pass it to a function outside ? Because when i do
...... BUM!

If you want to modify an object (in this case lista), then pass its
address; i.e. you need a registro *** parameter.
In addition, you have to admit the possibility of failure, so give
agregarElemento a return value indicating success or failure _and_
_check_ _it_!


Cheers
Michael
 
F

Flash Gordon

Michael said:
i have this little .c, it's in spanish but you'll get it anyway

#include <stdio.h>
typedef struct {
char cadena[20];
//creo una lista de 1 posicion
// i put a single element first
lista = (registro**) malloc (sizeof(registro*));
aux = (registro*) malloc(sizeof(registro));

The cast is unnecessary in C; in addition, this breaks
easily if you change the type.

You failed to mention that it hides the error of failing to include
stdlib.h which, from the code shown, is an error the OP has made.
> The best practice form around
here is
lista = malloc(sizeof *lista);
aux = malloc(sizeof *aux);
Of course you can put parentheses around *lista and *aux if
that makes you feel more comfortable.


Bad choice for input. Consider fgets() and strtol() instead -- they
give you much better information if it comes to finding errors.

Also, the way the OP used scanf is equivalent to using gets and allows
the user of the program to overflow the buffer.
If you have snprintf(), use it instead.

<snip>

Be aware that some implementations that provide snprintf as an extension
to C89 (the commonly implemented C standard) use different return values
to others.
 

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. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
474,177
Messages
2,570,953
Members
47,507
Latest member
codeguru31

Latest Threads

Top