Hello, OnlineGDB Q&A section lets you put your programming query to fellow community users. Asking a solution for whole assignment is strictly not allowed. You may ask for help where you are stuck. Try to add as much information as possible so that fellow users can know about your problem statement easily.

Let's get better in C - Exercise #3 - What's the problem with the following code?

+3 votes
asked Sep 21, 2020 by Peter Minarik (22,620 points)
edited Sep 28, 2020 by Peter Minarik


Thanks for the brilliant solution for xDELLx.
And thank you for everyone spending time to understand the problem and learn to be a better C programmer.
Well done!

UPDATE: the problem hasn't been solved yet. Anyone?

Note, if you need some more help, I'd suggest looking around the C string literals. ;)


This exercise is for anyone who would like to get better in C/C++ coding.

I've provided some sample code, that has some problem with it. (At least one, that I wanted to focus on in this exercise, but accidentally I could have made multiple mistakes. XD)

Let's see who finds the problem.

Please, tell me

  1. what part of the code has a problem
    • hint: if you run it, you'll find the error ;)
    • hint: the problem is not the Stack (if there is a problem with it, that wasn't my intention in this excerciuse)
  2. why it is a problem
    • hint: sure, it causes a crash, but what's the root cause? Why does the code crash?
  3. how would you fix it

The Code (with a problem):

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

// -----------------------------------------------------------------------------
// Stack structure and operations - BEGIN
// -----------------------------------------------------------------------------

// A Stack to store words
typedef struct __Stack
    struct __Stack * prev;
    const char * word;
} Stack;

// Pointer to the top of the stack
static Stack * stackTop = NULL;

// Push a new word on top of the stack
static void Stack_Push(const char * word)
    Stack * stackItem = (Stack*)malloc(sizeof(Stack));
    stackItem->prev = stackTop;
    stackItem->word = word;
    stackTop = stackItem;

// Pop the top word from the stack.
// Returns NULL when the stack is empty.
static const char * Stack_Pop()
    if (stackTop == NULL)
        return NULL;

    const char * word = stackTop->word;

    Stack * oldTop = stackTop;
    stackTop = stackTop->prev;
    return word;
// -----------------------------------------------------------------------------
// Stack structure and operations - END
// -----------------------------------------------------------------------------

// Print the words in the sentence in reverse order.
static void PrintSentenceReversed(char * s)
    printf("Original sentence: %s\n", s);
    for (char * word = strtok(s, " "); word; word = strtok(NULL, " "))

    printf("Sentence reversed:");
    for (const char * word = Stack_Pop(); word; word = Stack_Pop())
        printf(" %s", word);

int main()
    char s1[] = "My name is Bob.";
    char * s2 = "My name is Bob.";
    return 0;

4 Answers

+2 votes
answered Sep 26, 2020 by xDELLx (7,630 points)
selected Sep 28, 2020 by Peter Minarik
Best answer

Looks I am late to the party , but my 2 cents below.

CONCEPT : "We can manipulate data(ie do write operation on memory address) if and only if we own(memory address) accessible by us."

Now as Peter suggested , the core anamoly in the code is related the string literals.

They are special variables (compiler will decay them to special memory address) which are write-restricted & no change is allowed by "users".

Since compiler decayed string literal to special memory address(ie it allocated enough bytes somewhere in memory for all characters of the string & initialised them to correct values & added the terminating '\0' character and finally returns the base address of the memory chunk... [writing this statement makes me more indebted to compiler for making life easier !! ] )

If you made it this far ,I owe you an explanation of special address.To understand this best you need to dive deeper into the memory layout of C program. To digress, this memory loaction is not stack memory (like variable s1) ,nor heap memory(no new /malloc done )  ,global ,static.

Why is it write restricted ( ie not read only) , because in debug mode we can actually change contents of the literal, see below dummy program:

#include <stdio.h>

int main()
    char s1[] = "My name is Bob.";
    printf ("Stack s1 %s\n" , s1 );
    char * s2 = "My name is Bob.";
    printf ("Literal s2 %s\n" ,s2 );
    return 0;

/* ---- DEBUGGER output , see how s2 contents can be changed without seg fault in debug , but try s2[1]='e'  in code & see the code die ... :/

Reading symbols from a.out...done.
/usr/share/gdb/gdbinit: No such file or directory.
(gdb) b 11
Breakpoint 1 at 0x4005f7: file main.c, line 11.
(gdb) r             
Starting program: /home/a.out                                
Stack s1 My name is Bob.                 
Breakpoint 1, main () at main.c:11
11          printf ("Literal s2 %s\n" ,s2 );
(gdb) p s2              
$1 = 0x4006c1 "My name is Bob."
(gdb) set *(s2 +1)='e'                         
(gdb) p s2                 
$2 = 0x4006c1 "Me name is Bob."
(gdb) set *(s2+11)='T'                            
(gdb) p s2                    
$3 = 0x4006c1 "Me name is Tob."
(gdb) set *(s2+13)='m'                               
(gdb) p s2                       
$4 = 0x4006c1 "Me name is Tom."
(gdb) n
Literal s2 Me name is Tom.
13          return 0;


TL;DR: The string literal (below) is write restricted memory & changing it will causes Seg Fault.

char * s2 = "My name is Bob."; 

To Fix : dont write to memory which doesnt belong to us .

Hope this made sense. Cheers!!

commented Sep 28, 2020 by Peter Minarik (22,620 points)

char * s2 points to the string literal, which mustn't be changed by the user (violated by strtok()).
On the other hand char s1[] is a stack variable that *copied* the content of the original string literal (same as the one pointed to by s2). Hence we can modify it without restriction.

I usually resolve the problem by defining pointers to string literals always as "const char *" so the compiler will not allow others to modify it. (Unless someone force the compiler to do this, but then the joke is on them.)
0 votes
answered Sep 21, 2020 by Admin (4,400 points)
I see there is enough hint with in code itself, if we would run and try to debug.
commented Sep 21, 2020 by Peter Minarik (22,620 points)
Some of my previous "exercises"/"challenges" may have been more difficult that this one, considering those samples seemed to run fine at a quick glance.

At least this one fails with the given input. Easier to get more people engaged who learn C/C++.
+1 vote
answered Sep 21, 2020 by Lakshman (160 points)
1. The program crashes with Segmentation fault.

2. It is because of

           char * s2 = "My name is Bob.";
   char *s2 will place the "My name is Bob." in read-only memory and strtok tries to modify the s2 which is read-only causes segmentation fault.
commented Sep 21, 2020 by Peter Minarik (22,620 points)
edited Sep 22, 2020 by Peter Minarik
Getting close, but not exactly there yet.

The correct part: "strtok tries to modify the s2 which [...] causes segmentation fault."

The incorrect part: "char *s2 will place the "My name is Bob." in read-only memory "
>>>> char * is not read only.

"const char * s2" would prevent you from modifying of the content of the data pointer by s2.
"const char * const s2" would even prevent modifying where s2 points.

But that's not the case in my sample.

Keep digging. You're on the right path. strtok is only one key to the solution. Why does it work with the first input (s1) but not the second one (s2)?
0 votes
answered Sep 25, 2020 by Peter Minarik (22,620 points)
C'mon, give a go to this exercise. It's still not properly solved.

You can learn a lot from it.
Welcome to OnlineGDB Q&A, where you can ask questions related to programming and OnlineGDB IDE and and receive answers from other members of the community.