CODEWITHSUNDEEP
cpointerspass-by-referencescanf
user1327900
user1327900

Reputation:

Pass by address, scanf, de-referencing

I have written a program employing the following concept:

I create an integer x pass by address to a function, along with a filename, said function opens file if available, scans the first line and sets the value that pX points to equal to the scanned line.

Only it's not working, and I don't see what I'm doing wrong. As far as I can tell the code below is generally how one would accomplish it, but I'm not sure if I'm not using scanf() correctly with the pointer or what.

void foo() {
    char input[20] = "test.txt";
    int x = 1;
    bar(input, &x);
}

void bar(char *fileName, int *pX) {
    FILE *fp = fopen(fileName, "r");
    char *buffer = malloc(15 * sizeof(int));
    fgets(buffer, 15, fp);
    scanf(buffer, "%d", *pX);
    free(buffer);
    fclose(fp);
}

Upvotes: 1

Views: 1723

Answers (3)

chqrlie
chqrlie

Reputation: 145277

You use scanf() incorrectly: either use scanf directly to parse standard input or use sscanf() to parse the string read by fgets(). Furthermore, pX is already a pointer to int, which is what sscanf() expects to store the int value it converts, pass it directly: sscanf(buffer, "%d", pX);

Here is a modified version:

int bar(const char *fileName, int *pX) {
    char buffer[15];
    FILE *fp = fopen(fileName, "r");
    int success = 0;

    if (fp != NULL) {
        fgets(buffer, sizeof buffer, fp);
        if (sscanf(buffer, "%d", pX) == 1)
            success = 1;
        fclose(fp);
    }
    return success;
}

void foo(void) {
    int x = 1;
    bar("test.txt", &x);
    /* do something with x */
}

Notes:

  • allocating buf is unnecessary, just make it a local array with automatic storage.

  • char *buffer = malloc(15 * sizeof(int)); is incorrect: you allocate space for 15 int instead of 15 characters, which have a size of 1 by definition. Use the size of the destination type to avoid any inconsistencies:

    char *buffer = malloc(15 * sizeof(*buffer));

  • always check the return value of malloc() to avoid potential undefined behavior.

  • reading from fp without checking if fopen succeeded has potential undefined behavior.
  • the contents of the array pointed to by filename is not modified, make it a const char *.
  • it may be useful for bar to return a success indicator.
  • enable more warnings at compile time: gcc -Wall -Wextra -Werror or clang -Weverything -Werror might have caught the mistakes in scanf.

Upvotes: 1

Sourav Ghosh
Sourav Ghosh

Reputation: 134386

First of all, there's no pass-by-reference in C, function parameters are all passed by value. By passing a pointer-to-data, we make the simulation of the achieving the same effect as pass-by-reference, but that does not mean C has any pass-by-reference concept.

That said, the problem seems to be

 scanf(buffer, "%d", *pX);
                     ^^^^

where

  • the current syntax is invalid and invokes undefined behavior. Probably you need sscanf().

  • px is already a pointer to int. Passing px will be correct and suffice.

Moral of the story: enable compiler warnings and pay heed to them. They are there for a reason. With proper warnings enabled, you should see something like

warning: format %d expects argument of type int *, but argument 3 has type int [-Wformat=]

Finally,

  • Always check the return value of fopen() for success before using the file pointer.
  • Check for the return value of scanf() to ensure successful scanning.
  • Check for the return value of fgets() to ensure success

...basically, check return value of all library calls to make sure they worked as expected.

Upvotes: 1

Marievi
Marievi

Reputation: 5009

Change line :

scanf(buffer, "%d", *pX);

to :

sscanf(buffer, "%d", pX);

You need function sscanf for what you are trying to do.

Both scanf and sscanf take a pointer as an argument. pX is of type int *, therefore a pointer to an int and should work for you. What you pass, *pX, is the contents of this pointer, in other words an int.

Also, change line :

char *buffer = malloc(15 * sizeof(int));

to :

char *buffer = malloc(15 * sizeof(char));

or simply :

char *buffer = malloc(15);

and always check the result of malloc :

if (buffer == NULL){
    ...
}

Upvotes: 2

Related Questions