-4

Below is a snippet of my code. My code is supposed to reverse alphabet chars only and skip any special characters. I am currently trying to use "ab-cd" and hope to print "ba-cd", but when I run my code I get a bus error.

I have narrowed down the issue to lines 31 and 32. I malloc'd, but unsure why it will not let me switch chars.

Although it does not show, I will make sure to free the memory using free.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <cytype.h> // used for isalpha()

int main(int argc, const char *argv[])
{
    // declaring a pointer of type char to tempString
    char * tempString = (char *)malloc(sizeof(char)* 6);
    tempString = "ab-cd";

    char temp;
    temp = tempString[0];
    tempString[0] = tempString[1];
    tempString[1] = temp;

    printf("%s\n", tempString);

    return 0;
}
  • 1
    `tempString` points to a string literal. String literals are constants and cannot be changed. As a side note, the call to `malloc` is unnecessary and leads to a memory leak. – DYZ Jul 23 '21 at 18:32
  • 4
    Please post all code as text, not a picture of text. – dbush Jul 23 '21 at 18:33
  • 4
    `tempString = "ab-cd";` discards the pointer you got from `malloc`, and replaces it with a pointer to a string literal which isn't writable. You could copy the string `"ab-cd"` into the malloced space, if you allocated enough (currently you have allocated only enough for 1 character, and you need 6). – Nate Eldredge Jul 23 '21 at 18:33
  • The `malloc` isn't necessary. `char tempString[] = "ab-cd";` suffices. – Daniel Walker Jul 23 '21 at 18:56

2 Answers2

2

The line tempString = "ab-cd"; is not doing what you think it is. Just one line earlier you had stored in tempString the address of an allocated buffer. In this line, however, you store the address of "ab-cd" in tempString. You're not writing to the buffer but rather causing tempString to point somewhere else entirely.

Specifically, you're directing it a string literal which is stored in a read-only section of memory. Therefore, trying to alter it as you're doing is going to crash your program.

What you want is to write the string into the buffer you already have. This can be done by

strcpy(tempString, "ab-cd");

EDIT (thanks to @NateEldredge for catching this):

Your allocated buffer is not big enough to hold "ab-cd". You need to change the size of the allocation:

tempString = malloc(sizeof(char)*6);
Daniel Walker
  • 6,380
  • 5
  • 22
  • 45
  • 5
    You'd also need to increase the size of the malloc'ed buffer to at least 6. – Nate Eldredge Jul 23 '21 at 18:35
  • 1
    Yeah I was missing the * 6 in my malloc. Thank you. Also , your suggestions helped. I forgot that the way I was attributing tempString was making it a string literal. I used strcpy and saw the difference. Thank you for the help. It is greatly appreciated. Wishing good karma your way. – user14377467 Jul 23 '21 at 18:42
  • Code like `... malloc(sizeof(char)*6)` is a very bad idea, sorry no upvote from me. – Jabberwocky Jul 23 '21 at 20:29
  • I will endeavor to console myself. – Daniel Walker Aug 12 '21 at 03:14
2

For starters you allocated not enough memory to store the string "ab-cd"

char * tempString = (char *)malloc(sizeof(char));

You allocated only one byte that can store only one character.

Then in the following statement you reassigned the pointer tempString with the address of the first character of the string literal "ab-cd".

tempString = "ab-cd";

As a result the program produces a memory leak because the address of the allocated memory is lost.

Then you are trying to change the string literal pointed now by the pointer tempString. But any attempt to change a string literal results in undefined behavior. From the C Standard (6.4.5 String literals)

7 It is unspecified whether these arrays are distinct provided their elements have the appropriate values. If the program attempts to modify such an array, the behavior is undefined.

There is no need to allocate memory dynamically. You could just declare a character array and initialize it with the string literal like for example

char tempString[] = "ab-cd";

If you want to allocate an array dynamically you need to write

char *tempString = malloc( sizeof( "ab-cd" ) );
strcpy( tempString, "ab-cd" );

and then when the array will not be needed any more you should free it like

free( tempString );
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335