2

If a string is greater than a certain length, I want to truncate it to a certain length, and replace the last 3 characters with a period ("."). What I have right now causes a segmentation fault:

#define NAME_LENGTH 36

name is of type, char*. 

if (strlen(name) > NAME_LENGTH){
    //we need to truncate the name
    printf("NAME IS TOO LONG.. TRUNCATING\n");
    char *nCpy = NULL; //the truncated name
    strncpy(nCpy, name, NAME_LENGTH); //copy NAME_LENGTH number of characters from name into nCpy
    printf("Truncated name, now adding ...\n");
    strcat(name, "..."); //append "..." to end of name
    printf("... added, now copying to struct\n");
    strcpy(record->name, nCpy); //assign our name in our record
    printf("NAME IS NOW: %s\n", record->name);
}

Upon running, if a name that is longer than NAME_LENGTH, I get a segmentation fault.

Enter name > jjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjjj
NAME IS TOO LONG.. TRUNCATING
Segmentation fault (core dumped)
Talen Kylon
  • 1,908
  • 7
  • 32
  • 60

2 Answers2

3

You are segfaulting because you didn't allocate memory to store where nCpy is pointing at.

char *nCpy = NULL; //the truncated name

Should be something like

char *nCpy = malloc(sizeof(char) * NAME_LENGTH + 1); //the truncated name

What you're trying now is to write to some junk value in memory, who knows, which almost always leads to a segmentation fault.

As Paul points out, you need to allocate space for NAME_LENGTH characters, plus one since a character string is null terminated with the special /0 character. This specific error is called dereferencing a null pointer

Community
  • 1
  • 1
edwardmp
  • 6,339
  • 5
  • 50
  • 77
  • 2
    You'll want to allocate `NAME_LENGTH + 1` chars; `strncpy()` won't null-terminate if it runs out of room. – Paul Roub Nov 16 '13 at 21:53
  • @edwardmp, thanks for your suggestion. Is it appropriate to free(nCpy) once I have assigned it to record->name? – Talen Kylon Nov 16 '13 at 21:54
  • 1
    @FatalProphet yes I probably think you can free it after that line. But you'll notice it yourself when you play it with ;) – edwardmp Nov 16 '13 at 21:55
0

You are really doing far too many string copies, and not freeing the copies afterwards, so you're going to end up with a lot of leaked memory. Also, it's not clear whether record->name is actually long enough to hold the returned string, but if it is, you might as well build the name in place.

Here's one possibility, which assumes that record->name already points to at least NAME_LENGTH + 1 bytes of storage:

if (strlen(name) > NAME_LENGTH)
  snprintf(record->name, NAME_LENGTH + 1, "%.*s...", NAME_LENGTH - 3, name);
else
  strncpy(record->name, name, NAME_LENGTH + 1);

Here's another, possibly simpler or possibly more mysterious way of doing it:

if (snprintf(record->name, NAME_LENGTH + 1, "%s", name) > NAME_LENGTH))
  strcpy(record->name + NAME_LENGTH - 3, "...");
rici
  • 234,347
  • 28
  • 237
  • 341