Allocating memory to an array and changing a value in

Question

I have written very simple code, that I used to feel confident in but it isn't working. I don't know if I am doing anything wrong or perhaps it is something wrong with my vscode. I have allocated memory for an array of integers and chars, and I then use scanf to read values that I then want to save in said arrays. But it appears it is saving the values as garbage value when I use printfs to debug.

For context: tamanhos = sizes, which are supposed to be either p, P, g or G, and quantidades are supposed to be integers.

#include <stdio.h>
#include <stdlib.h>
#define professores 7
int main () {
    int quantidade, total, holder;
    char tamanho;

    total = 0;

    int * quantidades = (int*)malloc(professores*sizeof(int));
    char * tamanhos = (char*)malloc(professores*sizeof(char));

    for (int i = 0; i < 7; i++){
        scanf(" %d", &quantidade);
        scanf(" %c", &tamanho);
        quantidades[i] = quantidade;
        tamanhos[i] = tamanho;
    }

    printf("%d", total);

    for (int i = 0; i < 7; i++ ){
        if (tamanhos[i] == "P" || tamanhos[i] == "p") {
            holder = quantidades[i];
            total = total + quantidades[i]*10;
        }
        else if (tamanhos[i] == "G" || tamanhos[i] == "g") {
            total = total + quantidades[i]*16;
        }
    }
    printf("%d\n", total);
    int xprof = total / 7;
    printf("%d", xprof);

    free(quantidades);
    free(tamanhos);

    return 0;
}

Side note, use `professores` instead of hard coding `7` in the loop controls and divisor. — Weather Vane, Aug 22 '21 at 17:24
Also avoid casting the output of malloc (cf https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Foxy, Aug 22 '21 at 17:24
I see the problem with your code. Your scanf statements are using the & operator. The result is instead of *quantidades, you're getting **quantidades which is a pointer to a pointer. Remove the & and it should work properly. The & is the address of operator and it provides the memory address of the associated item. Since your variables are already pointers, there's no need to use it. — Daniel Rudy, Aug 23 '21 at 04:35

score 2 · Answer 1 · answered Aug 22 '21 at 17:53

In C and in C++ single quotes identify a single character, while double quotes create a string literal. You are compairing character so use single quotes instesd of double.

#include <stdio.h>
#include <stdlib.h>
#define professores 7
int main () {
    int quantidade, total = 0, holder;
    char tamanho;    
    int * quantidades = (int*)malloc(professores*sizeof(int));
    char * tamanhos = (char*)malloc(professores*sizeof(char));

    for (int i = 0; i < 7; i++){
        scanf(" %d", &quantidade);
        scanf(" %c", &tamanho);
        quantidades[i] = quantidade;
        tamanhos[i] = tamanho;
    }
    printf("%d\n", total);
    for (int i = 0; i < 7; i++ ){
        if (tamanhos[i] == 'P' || tamanhos[i] == 'p') {
            holder = quantidades[i];
            total = total + quantidades[i]*10;
        }
        else if (tamanhos[i] == 'G' || tamanhos[i] == 'g') 
            total = total + quantidades[i]*16;
    }
    printf("%d\n", total);
    int xprof = total / 7;
    printf("%d", xprof);

    free(quantidades);
    free(tamanhos);
    return 0;
}

score 2 · Answer 2 · edited Aug 23 '21 at 06:45

In your code here...

scanf(" %d", &quantidade);
scanf(" %c", &tamanho);

don't use spaces in scanf(" %d, &quantidade);

and also you can insert values in the array directly like...

scanf("%d", &quantidades[i]);
scanf("%c", &tamanhos[i]);

If I am not wrong then your compiler must be giving you a warning here...

if (tamanhos[i] == "P" || tamanhos[i] == "p") {

and here..

else if (tamanhos[i] == "G" || tamanhos[i] == "g") {

You are comparing a single character here only so use only single colon...

Yun · Accepted Answer · 2021-09-24T22:13:42.460

1

Main problem:

tamanhos[i] == "P" compares a char to "pointer to a string literal". You probably meant to use single quotes, i.e. tamanhos[i] == 'P'.

Minor points of improvement:

int main() is, generally, not a valid signature for this function, use int main(void) when not using the parameters.
The magic number 7 appears several times. Perhaps these should be replaced by the macro professores? The latter is commonly written in all caps to distinguish it from a regular variable.
A value is stored in holder, but this is never read.
Try to declare and initialize variables in one statement (e.g. int n = 5), unless you are using an old version of C which does not allow this.
Try to make the scope of the variables as small as possible. Simply put, declare them directly before they are used.
The function scanf can be tricky to use w.r.t. avoiding buffer overflows. It is best replaced by a combination of fgets followed by sscanf (or parsing the result manually).
You can write values directly to an array, e.g. scanf(" %d", &quantidades[i]);.

Additional credits: Oka and Andrew Henle.

edited Sep 24 '21 at 22:13

answered Aug 22 '21 at 17:44

Yun

3,056
6
9
28

Thanks a lot for your feedback. What do you recommend using instead of scanf? – JOAO VITOR PEREIRA VENTURA Aug 22 '21 at 17:51
@JOAOVITORPEREIRAVENTURA You're welcome. Search for `scanf` [here](https://stackoverflow.com/questions/1253053/what-makes-a-c-standard-library-function-dangerous-and-what-is-the-alternative) for an alternative and broader discussion. If you must use `scanf`, then it's best to also check its return value to see if the read was successful. – Yun Aug 22 '21 at 17:59
1

Calling `scanf` *generally not safe* is misleading. Some precautions must be taken to limit input lengths when attempting to read strings (`%s`, `%[]`), but mainly `scanf` is just very tricky to use, rather than unsafe, with many pitfalls surrounding common tasks. The most commonly suggested alternative is to read partial/entire lines with `fgets`, and parse them manually or with `sscanf`, which gives you more robust control when parsing and handling bad input. – Oka Aug 22 '21 at 18:14
@Oka True, true. It's not _always_ unsafe, but there's a reason it's considered deprecated in C11 by, e.g., Clang's analyzer. I'll update the answer. – Yun Aug 22 '21 at 18:32
2

*In C11, it is best replaced by `scanf_s`...* [IMO, no it should not be](http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm), unless you want to write *de facto* non-portable code that is not any more secure. The real problem with `scanf()` is that it leaves your input stream in an unknown state if a failure occurs so `fgets()` then `sscanf()` is IMO the only real solution. – Andrew Henle Aug 22 '21 at 19:23
@AndrewHenle Thank you. I was under the impression that these functions were [part of C11](https://en.cppreference.com/w/c/io/fscanf), but I now see that it is only optional and poorly supported. I'll update the answer. – Yun Aug 22 '21 at 19:38

Allocating memory to an array and changing a value in

3 Answers3