pointer of int calculate average

Question

i have a problem in the result of the average in the function calcular, not return a correct number, and the rand return values equals, i dont know the reason for the bad average

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

int calcular(int * _arreglo, int * _tam);

int main(int argc, char * argv[]) {
    int tam = 0;
    if (argc == 2) {
        tam = atoi(argv[1]);
    } else {
        printf("No se ha ingresado correctamente el tamaño \n");
        exit(1);
    }
    int * arreglo;
    arreglo = (int * ) malloc(sizeof(int) * tam);
    int i;
    for (i = 0; i < tam; i++) {
        arreglo = (int * )(rand() % 101);
        printf("soy %d \n", ((int)(arreglo)));
        arreglo++;
    }
    int promedio = calcular(arreglo, & tam);
    printf("Promedio: %d \n", promedio);
    free(arreglo);
    return 0;
}

int calcular(int * _arreglo, int * _tam) {
    int pro = 0;
    int i;
    _arreglo = _arreglo - ( * _tam);
    for (i = 0; i < * _tam; i++) {
        pro = pro + ((int)(_arreglo));
        _arreglo++;
    }
    return (pro / ( * _tam));
}

the principal problem is the average in the function calcular, i think is a problem with the int pointer — J Mateo Pineda Lop, Oct 04 '15 at 14:21
@cad Calling `srand()` is not needed. That is only useful if code needs to have a _different_ simulation. Until the first simulation works as intended, _not_ including `srand()` simplifies debugging. — chux - Reinstate Monica, Oct 04 '15 at 22:03

score 3 · Answer 1 · edited May 23 '17 at 12:06

3

Main issues:

This:

for(i=0;i<tam;i++){
arreglo=(int *)(rand()%101);
printf("soy %d \n",((int)(arreglo)));
arreglo++;
}

doesn't make any sense. You probably wanted

for(i = 0; i < tam; i++) {
    arreglo[i] = (rand() % 101);
    printf("soy %d \n", arreglo[i]);
}

Here:

int calcular(int *_arreglo, int *_tam){
int pro=0;
int i;
_arreglo=_arreglo-(*_tam);
for(i=0;i<*_tam;i++){
pro=pro+((int)(_arreglo));
_arreglo++;
}
return (pro/(*_tam));
}

you probably wanted

int calcular(int *_arreglo, int *_tam){
    int pro = 0;
    int i;

    for(i = 0; i < *_tam; i++){
        pro = pro + _arreglo[i];
    }

    return (pro / (*_tam));
}

Other issues:

You need to call seed rand for getting a different set of random values on each run of the program so that you don't get the same set of random numbers on each run of the program. Add srand(time(NULL)); at the start of main after including time.h. This will return a different set of random numbers, provided that the program isn't run more than one time in the same second.
It doesn't make sense why you are passing tam by reference into calcular. Pass it by value instead.
There is no need cast the result of malloc (and family) in C.

Consider checking the return value of malloc to see if it was successful. Change:

arreglo = (int * ) malloc(sizeof(int) * tam);

to

if((arreglo = malloc(sizeof(int) * tam)) == NULL) /* If malloc failed */
{
    fputs("malloc failed; Exiting...", stderr); /* Print the error message in the `stderr` */
    exit(-1); /* Exit the program */
}

edited May 23 '17 at 12:06

Community

1
1

answered Oct 04 '15 at 14:20

Spikatrix

20,225
7
37
83

`man rand: [...]If no seed value is provided, the rand() function is automatically seeded with a value of 1.[...]`. – EOF Oct 04 '15 at 14:23
But seeding with 1 returns the same values on each run of the program, right? – Spikatrix Oct 04 '15 at 14:27
@CoolGuy The same seed will always return the same sequence of pseudo-random numbers. That's why that Microsoft card game (I don't remember which one, it's been a very long time) ... asked for a number to play the same game, it is the seed for the generator. – Iharob Al Asimi Oct 04 '15 at 14:29
1

Sure, but calling `srand(time(0))` causes `rand()` to return the same sequence when the program is run again in the same second. – EOF Oct 04 '15 at 14:29
@EOF, Exactly. Because of what I say in my previous comment. – Iharob Al Asimi Oct 04 '15 at 14:29
@EOF Edited the answer. I hope it looks OK now. – Spikatrix Oct 04 '15 at 14:38
Minor: Consider 1) `fputs()` rather than `printf("malloc failed; Exiting...");` for the usual `'%'` concerns. 2) `fputs(..., stderr)` for error messages. – chux - Reinstate Monica Oct 04 '15 at 17:08
@iharob "that Microsoft card game " --> [freecell](https://en.wikipedia.org/wiki/FreeCell)? – chux - Reinstate Monica Oct 04 '15 at 17:11
@chux exactly, that one. – Iharob Al Asimi Oct 04 '15 at 21:57

Iharob Al Asimi · Answer 2 · 2015-10-04T14:26:04.323

You have a serious mistake

arreglo = (int *) (rand() % 101);

is assigning a random address to the pointer arreglo, dereferencing it is undefined behavior.

Reassigning the pointer after malloc() will cause a memory leak, because now you can't free() the pointer returned by malloc(). Moreover, you free() the pointer with the invalid address assigned with rand(), and that is undefined behavior.

Some other considerations

Make your code readable to humans
- Surround operatrs with white spaces, it adds a lot of clarity to the code because otherwise it becomes difficult to differentiate between tokens.
- Indent your code correctly.
You don't need to cast malloc(), this also improves readability.
Check the pointer returned by malloc() for NULL, on error malloc() returns NULL, if you dereference a NULL pointer undefined behavior will occur.

Use strtol() instead of atoi() to check for invalid input, like this

char *endptr;
tam = strtol(argv[1], &endptr, 10);
if (*endptr != '\0')
    return -1; /* invalid input */

pointer of int calculate average

2 Answers2

Main issues:

Other issues: