0

I am a C beginner who has been assigned to write a program that uses pointers to reverse a message. I am having trouble getting the for loop that reads the characters to break after it reads a newline and I don't want to use a while loop.

Below is my code:

#include <stdio.h>
#include <string.h>
int main(){
        //declare string
        char reverse[100];

        //declare pointer
        char *first;

        //set pointer to point to first element of array
        first = &reverse[0];

        //get chars until end of input  
        printf("Enter a message:");
        for (first = reverse; *first != '\n'; first++){
                scanf("%c", first);
                printf("%c", *first);
        }

        //reverse chars one by one
        printf("Reversal: ");
        for (first; first >= reverse; first--){
                printf("%c", *first);
        }
        printf("\n");


        return 0;
}

Thank you! Any help is appreciated :)

Renee Amos
  • 11
  • 3
  • Don't use `scanf` to read a single char, use `getchar`. (Really, don't use `scanf` for anything.) – William Pursell May 26 '22 at 03:03
  • `reverse` is not initialized. So the first time you read `*first` in the check of the for loop, you have undefined behavior. – William Pursell May 26 '22 at 03:05
  • 2
    Note that you `scanf` into `*first`, and then you increment `first`, and then you check if the value of `*first` is `\n`. But the value you are checking is *not* the one you just wrote, because you incremented the pointer. Also, you *must* check the value returned by `scanf`. You *could* do `for( first = reverse; scanf("%c", first) == 1 && *first != '\n' && first - reverse < sizeof reverse; first += 1 ) ...`, but it is much more idiomatic to use `getchar`. – William Pursell May 26 '22 at 03:12
  • @WilliamPursell Thanks for the input. I am supposed to use scanf and implement this function as part of an assignment. I implemented the code you specified ```(for( first = reverse; scanf("%c", first) == 1 && *first != '\n' && first - reverse < sizeof reverse; first += 1 ) ...)```, but it produced strange output. Sorry for the ignorant question, but how would I actually initialize char reverse[100]; to prevent undefined behavior? – Renee Amos May 26 '22 at 03:37
  • Initiailization can be as easy as `char reverse[100] = {0}` or `char reverse[100] = "";`. But the better way to avoid the UB is to *not* look at entries in the array that have not yet been assigned. – William Pursell May 26 '22 at 11:27
  • If you are getting "strange output", perhaps you are still doing `scanf` inside the loop body. Need to see code. – William Pursell May 26 '22 at 11:28

1 Answers1

0

You have effectively implemented gets, a function so dangerous it was removed from the language, with a side of undefined behaviour because *first != '\n' is executed at the start of each iteration of the loop, and *first is an indeterminate value at this point.

first-- and first >= reverse are also a problem as holding a pointer value to one before the start of an object is also undefined behaviour. It is valid C to hold (but not use with unary *) an address one past the end of an array object (C11 §6.5.6).

Note, using getchar would be preferable to scanf.

If you do not want the newline in the buffer:

#include <stdio.h>

#define SIZE 100

int main(void) {
    char buffer[SIZE];
    char *dest = buffer;

    for (; dest - buffer < sizeof buffer - 1; dest++)
        if (1 != scanf("%c", dest) || '\n' == *dest)
            break;

    *dest = 0;

    while (dest > buffer)
        putchar(*--dest);

    putchar('\n');
}

If you want the newline in the buffer:

#include <stdio.h>

#define SIZE 100

int main(void) {
    char buffer[SIZE];
    char *dest = buffer;

    for (; dest - buffer < sizeof buffer - 1;)
        if (1 != scanf("%c", dest) || '\n' == *dest++)
            break;

    *dest = 0;

    while (dest > buffer)
        putchar(*--dest);

    putchar('\n');
}

Note the difference in when dest is incremented.

while (dest > buffer)
    putchar(*--dest);

works by starting at the null-terminating character, and printing each element that comes before the current one, until our current element is the start of the array.


This can also get extremely terse, and you may be tempted to write something like

/* newline in buffer */
for (; dest - buffer < sizeof buffer - 1 && 1 == scanf("%c", dest) && '\n' != *dest; dest++);

but use of the break keyword can help to keep it readable.

Oka
  • 23,367
  • 6
  • 42
  • 53