String operations cause segfault C

Question

I'm trying to 'deep copy' a string so that I can perform operations on one copy, while retaining the original copy. This is the base example that I've got and for some reason the strncpy call causes a segfault. Please help

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

int main() {
    char* stringA = "someVeryinTeresTingString";
    char* stringB = malloc(sizeof(char) * strlen(stringA));
    
    printf("A: %s, B: %s\n", stringA, stringB);
    
    for (int i = 0; i < strlen(stringA); i++) {
        stringB[i] = tolower(stringA[i]);
    }
    
    printf("A: %s, B: %s\n", stringA, stringB);
    
    strncpy(stringA, stringB, strlen(stringA) - 1);
    
    printf("A: %s, B: %s\n", stringA, stringB);
}

Hint: Don't forget the NUL terminator. Don't forget `strdup()` if you have it. — tadman, Mar 16 '21 at 10:02
`malloc()` may return junk data. `stringB` is not a valid C string until you clear it or put something in it. — tadman, Mar 16 '21 at 10:03
`stringA` is also not mutable, it's a static string and should be considered to be `const char*`. You need a buffer you can write to, so allocate a new one, or as mentioned, just use `strdup()`. — tadman, Mar 16 '21 at 10:04
@tadman so if I wanted to write a method to return the lowercase of a string, I'd have to create a new char*, dump the contents into that, free the pointer passed in and return the new char* pointer? — flying_loaf_3, Mar 16 '21 at 10:06
Yeah. Inline string literals are special in that they're not stored in mutable memory, they're effectively immutable. You need to copy them, so you can do `char stringA[] = "..."` which creates a local array, not a pointer, or you can `strdup()` to create a copy. — tadman, Mar 16 '21 at 10:08
@tadman, but in this SO post here, they are editing the ith index of a string, which implies that string are somewhat immutable? https://stackoverflow.com/questions/2661766/how-do-i-lowercase-a-string-in-c — flying_loaf_3, Mar 16 '21 at 10:19
C strings on the stack or heap *are* mutable. [*C string literals*](https://en.cppreference.com/w/c/language/string_literal) are not. — tadman, Mar 16 '21 at 10:20

tadman · Accepted Answer · 2021-03-16T10:14:02.120

1

Easiest fix is to make a local copy of that string literal:

char stringA[] = "someVeryinTeresTingString";

Everything else works just the same.

Note that in the original code you have a pointer to immutable memory, while in this version you have a local (stack) array that is initialized with a copy of that string.

Another thing to note is if you're copying and manipulating C strings, do things like this:

char* stringB = strdup(stringA);
  
for (int i = 0; i < strlen(stringB); ++i) {
    stringB[i] = tolower(stringB[i]);
}

Or even more efficiently by avoiding all these expensive strlen() calls:

char* stringB = strdup(stringA);
  
for (char* p = stringB; *p; ++p) {
    *p = tolower(*p);
}

edited Mar 16 '21 at 10:14

answered Mar 16 '21 at 10:10

tadman

208,517
23
234
262

Hmm `printf("B: %s\n", stringB);` remains UB as `stringB` is not a _string_. – chux - Reinstate Monica Mar 16 '21 at 10:11
@chux-ReinstateMonica I mentioned that in a comment. It's not touched on here. – tadman Mar 16 '21 at 10:11
All 3 `printf("B: %s\n", stringB);` are UB as `stringB` lacks a null character or space for it. – chux - Reinstate Monica Mar 16 '21 at 10:14
@chux-ReinstateMonica I'm recommending `strdup()` here in place of the original code. That solves that issue. – tadman Mar 16 '21 at 10:14
Fair enough.... – chux - Reinstate Monica Mar 16 '21 at 10:14
Please note that `strdup` is not C standard and non-portable. Though I heard some rumour about adding it to C2x. – Lundin Mar 18 '21 at 09:00
1

@Lundin Some things in POSIX were admittedly a mistake, like `gets()`, but I'm not sure `stdrup()` is one of them. – tadman Mar 18 '21 at 10:35
1

anyway, if `strdup` is missing in your setup, coding it is pretty trivial. Everywhere else (99.9% of the time) it exists. – Jean-François Fabre Mar 18 '21 at 10:40

Asaf Itach · Answer 2 · 2021-03-16T10:19:32.677

0

This line:

char* stringB = malloc(sizeof(char) * strlen(stringA));

shuld be like this:

char* stringB = malloc(sizeof(char) * (strlen(stringA) + 1));

then you will able to copy the \0 in the end of stringA

also, you want to copy to literal string - that is segmentation fault

char *strncpy(char *dest, const char *src, size_t n)

edited Mar 16 '21 at 10:19

answered Mar 16 '21 at 10:11

Asaf Itach

300
6
13

Why can't I copy a string literal? – flying_loaf_3 Mar 16 '21 at 10:19
you can copy string literal but you cant copy t-o string literal. you can switch stringA and stringB in the strncpy function and it will run – Asaf Itach Mar 16 '21 at 10:20
Ah yeah I forgot that strlen(stringA) is the length of stringA without the \0 char – flying_loaf_3 Mar 16 '21 at 10:24

Luis Colorado · Answer 3 · 2021-03-18T09:11:12.863

I'll try to comment and correct in your own code the mistakes I've seen:

(I will not correct things that can be eliminated or better done in another way, but are correct or not harmful, so you'll see only what must be corrected because of programming errors, and not questions about style or programming uses)

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

int main() {
    char* stringA = "someVeryinTeresTingString";

    /* you need to consider the space for the final null character in the malloc() call */
    char* stringB = malloc(sizeof(char) * (strlen(stringA) + 1));
    /* you don't need to use sizeof(char) as it is always equal to one.
     * Multiplying by one is not necessary, but you'll probably know.
     * char is warranteed by C standard that its sizeof is one. */

    /* you need to copy the string *before* printing, or you will print an 
     * uninitialized string.  Or at least initialize stringB to zeros, so you can
     * use it with printf like functions  (I do initialize the first char position to
     * zero to make it appear as a length zero "" string)
     * You will incurr in undefined behaviour if you don't do this. */
    stringB[0] = '\0';
    
    printf("A: %s, B: %s\n", stringA, stringB);
    
    /* you need to copy the strings, so you can do it better if you test when
     * stringA[i] == '\0', so you don't calculate the length of a string that is
     * not going to change at every loop iteration.  I will not change your
     * code, because this is not an error.  But strlen() searches from the
     * beginning of the string for the '\0' char, character by character,
     * and this test is done at every loop iteration.  With the expression
     * stringA[i] == 0 you do only a test per loop iteration to see if
     * the char at position i in stringA is the null character. */
    int i;
    for (i = 0; i < strlen(stringA); i++) {
        stringB[i] = tolower(stringA[i]);
    }
    /* you have not copied the final '\0', so I do it now.  I need to move the
     * declaration of i outside of the loop to be able to use it's value. */
    stringB[i] = 0;  /* you can use 0 or '\0' interchangeably */
    
    printf("A: %s, B: %s\n", stringA, stringB);
    
    /* nope.  you need to copy the strings with a normal strcpy() as you know that
     * both are the same length  (better, you know that the space in stringB
     * is the same as the length of stringA plus one).  If you do this, you will not copy the last '\0' char, so wee need to append it.
     * well, I don't know if that is what you want, so I don't actually touch anything here. */
    strncpy(stringA, stringB, strlen(stringA) - 1);
    
    /* stringB should be one char shorter than stringA */
    printf("A: %s, B: %s\n", stringA, stringB);
}

by the way, you have been recommended to use strdup(3). This is a good idea, you don't need to be thinking on final nulls in this case, because strdup() takes care of it. Just remember that strdup(3) is not included in many C standard revisions, so you can get in trouble if you move your program to a place lacking it (that should be very strange, anyway)

String operations cause segfault C

3 Answers3