I am trying to write a program that will dynamically allocate enough space to store all the words in a 1D char array separated by a space. ex:

char *literal = "The quick brown fox";
char **words = { "The", "quick", "brown", "fox" };

The program I wrote keeps segfaulting when trying to strncpy(str[buff_ptr],tok,strlen(tok));

I will post my code bellow:

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

char *mutableString(char *lit) {
    int size = strlen(lit);
    char *str = (char *)malloc(sizeof(char) * size);
    strncpy(str, lit, size   1);
    return str;
}

int numTokens(char *str, const char *DELIM) {
    char* clone = (char*)malloc(sizeof(char*));
    strncpy(clone, str, strlen(str)   1);
    int count = 0;
    for (char *tok = strtok(clone, " "); tok != NULL; tok = strtok(NULL, " "))
        count  ;
    free(clone);
    return count;
}

char **tokenize(char *str, const char *DELIM) {
    printf("tokenize-------------------------\n");
    int size = numTokens(str, DELIM);
    //allocate space on heap for buffer
    char **buff = (char **)malloc(size * sizeof(char *));
    //get first word
    char *tok = strtok(str, DELIM);
    int buff_ptr = 0;
    while (tok != NULL) {
        strncpy(buff[buff_ptr], tok, strlen(tok)   1);
        printf("buff[%d]%s\n", buff_ptr, buff[buff_ptr]);
        //increment to next word for storage
        buff_ptr  ;
        //find next word in string
        tok = strtok(NULL, DELIM);
    }
    for (int i = 0; i < size; i  ) {
        printf("%s\n", buff[i]);
    }
    //return 2D pointer
    return buff;
}

int main() {
    char *literal = "some literal string.";
    //convert string to mutable string for strtok
    char *str = mutableString(literal);
    //set 2D pointer equal to the pointer address returned
    char **no_spaces_str = tokenize(str, " ");
    printf("%s\n", str);
    for (int i = 0; i < numTokens(str, " "); i  ) {
        printf("%s\n", no_spaces_str[i]);
    }
    //free heap allocated memory
    free(str);
    free(no_spaces_str);
    return 0;
}

Please see attachment of lldb stack variables:

CodePudding user response：

Within the function mutableString there is dynamically allocated the character array str that does not contain a string

char* mutableString(char* lit){
  int size = strlen(lit);
  char* str = (char*)malloc(sizeof(char)*size);
  strncpy(str,lit,size);
  return str;
}

So other functions invoke undefined behavior as for example in this for loop

int numTokens(char* str, const char* DELIM){
  int count = 0;
  for(; *str != '\0'; str  )
  //...

Moreover if the array contained a string nevertheless the function numTokens is incorrect because for example it returns 0 when a passed string contains only one word.

Also in the function tokenize

strncpy(buff[buff_ptr],tok,strlen(tok));

there are used uninitialized pointers buff[buff_ptr] allocated like.

char **buff = (char**)malloc(size*sizeof(char*));

And again you are trying to copy strings without including the terminating zero character '\0; using eth functions strncpy.

So this call in main

printf("%s\n",no_spaces_str[i]);

also will invoke undefined behavior.

CodePudding user response：

Apart from the @Vlad from Moscow mentioned points,

1. Code don't have equal number of free operations, hence there is a memory leak. refer
2. strtok returns the pointer of the position after delimiters if not found it will return null. strok
3. malloc return values must not be type-casted Do I cast the result of malloc?

I tried to clean up the code find the snippet below, DEMO

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

typedef struct{
    char** buff;
    int    size;
}Array_2d;

char* mutableString(const char* lit){
  int size = strlen(lit);
  char* str = malloc(size);
  strncpy(str,lit,size 1);
  return str;
}

int getNextWordLength(const char* str){
  int index = 0;
  while(*str && (*str != ' ')){
      //printf("%c",*str);
        index;
        str;
  }
  return index;
}

int numTokens(const char* str){
  int count = 0;
  for(; *str != '\0'; str  )
  {
    if(*str == ' ')
      count  ;
  }
  return count;
}

void tokenize(const char* str, const char *DELIM, Array_2d *array){

  int len = strlen(str) 1;

    if(!str && !len){
        array->buff = 0;
        array->size = 0;
    }

    int number_of_words = numTokens(str) 1;
    //allocate space on heap for buffer
    char **buff = (char**)malloc(number_of_words*sizeof(char*));

    int index = 0; 

    do{
        //get first word
        
        int word_length = getNextWordLength(str);
        
        //To compensate null terminal
        buff[index] = malloc(word_length 1);

        strncpy(buff[index], str,word_length);

        buff[index][word_length 1] = '\0';

        str  = word_length 1;
          index;

    }while(index < number_of_words);

    //update return value
    array->buff = buff;
    array->size = number_of_words;

}

int main(){
    char* literal = "hello world this is test";
    //convert string to mutatable string for strtok
    char* str = mutableString(literal);
    printf("Complete String is : %s\n",str);
    

    Array_2d array;
    // set 2D pointer equal to the pointer addres returned
     tokenize(str, " ",&array);

    printf("Tokenized String\n");
    for(int i=0;i<array.size;i  ){
        printf("%s\n",array.buff[i]);
    }

    free(str);

    for(int i =0;i< array.size;   i)
    free(array.buff[i]);

    free(array.buff);

  return 0;
}

CodePudding user response：

char* mutableString(char* lit){
  int size = strlen(lit);
  char* str = (char*)malloc(sizeof(char)*size);
  strncpy(str,lit,size 1);
  return str;
}
int numTokens(char* str, const char* DELIM){
  char* clone = (char*)malloc(sizeof(char)*strlen(str));
  strncpy(clone,str,strlen(str) 1);
  int count = 0;
  for(char* tok = strtok(clone," "); tok != NULL; tok=strtok(NULL, " "))
      count  ;
  free(clone);
  return count;
}
char** tokenize(char* str, const char* DELIM){
  char* clone = (char*)malloc(sizeof(char)*strlen(str));
  strncpy(clone,str,strlen(str) 1);
  // printf("tokenize-------------------------\n");
  int size = numTokens(str, DELIM);
  //allocate space on heap for buffer
  char **buff = (char**)calloc(size,sizeof(char*));
  //get first word
  char* tok = strtok(clone,DELIM);
  int buff_ptr = 0;
  while(tok != NULL){
    // printf("token%d:%s\n",buff_ptr,tok);
    buff[buff_ptr] = (char*)malloc(sizeof(char)*strlen(tok) 1);
    strncpy(buff[buff_ptr],tok,strlen(tok) 1);
    //increment to next word for storage
    buff_ptr  ;
    //find next word in string
    tok = strtok(NULL, DELIM);
  }
  //return 2D pointer
  free(clone);
  return buff;
}
int main(){
  char* literal = "some literal string.";
  //convert string to mutatable string for strtok
  char* str = mutableString(literal);
  //set 2D pointer equal to the pointer addres returned
  char** no_spaces_str = tokenize(str, " ");
  int num_words = numTokens(str," ");
  char* oneD = (char*)calloc(strlen(str) 1,sizeof(char));
  for(int i = 0;i<num_words;i  ){
    strncat(oneD,no_spaces_str[i],strlen(no_spaces_str[i]) 1);
    printf("%s\n",oneD);
  }
  
  //free heap allocated memory
  free(str);
  free(no_spaces_str);
  free(oneD);
  return 0;
}

Is solution to my problem. Thanks to all those who commented and helped me understand dynamic memory better.

CodePudding user response：

This is the corrected version of the code above

1. When you copying string you should add 1 char for '\0'

   int size = strlen(lit) 1;

2. Tokens buffer size should be size 1

   int size = numTokens(str, DELIM) 1;

3. Strncpy is not required strncpy(buff[buff_ptr], tok, strlen(tok) 1); you already copied string char* str = mutableString(literal); just point to n-th buffer every next token buff[buff_ptr]=tok;

4. for (int i = 0; i<numTokens(str, " "); i  ){
       printf("%s\n", no_spaces_str[i]);
   }
   

   This code wouldn't work correctly. strtok manipulates the string you pass in and returns a pointer to it, so no memory is allocated. so all spaces will be replaced by '\0'

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

#pragma warning(push)
#pragma warning(disable : 4996)

char* mutableString(char* lit){
    int size = strlen(lit) 1;
    char* str = (char*)malloc(sizeof(char)*size);
    strncpy(str, lit, size);
    return str;
}
int numTokens(char* str, const char* DELIM){
    int count = 0;
    for (; *str != '\0'; str  )
    {
        if (*str == ' ')
            count  ;
    }
    return count;
}
char** tokenize(char* str, const char* DELIM){
    printf("tokenize-------------------------\n");
    int size = numTokens(str, DELIM) 1;
    //allocate space on heap for buffer
    char **buff = (char**)malloc((size)*sizeof(char*));
    //get first word
    char* tok = strtok(str, DELIM);
    int buff_ptr = 0;
    while (tok != NULL){

        buff[buff_ptr]=tok;
        printf("buff[%d]%s\n", buff_ptr, buff[buff_ptr]);
        //increment to next word for storage
        buff_ptr  ;
        //find next word in string
        tok = strtok(NULL, DELIM);
    }

    for (int i = 0; i<size; i  ){
        printf("%s\n", buff[i]);
    }
    //return 2D pointer
    return buff;
}
int main(){
    char* literal = "some literal string.";
    //convert string to mutatable string for strtok
    char* str = mutableString(literal);
    //set 2D pointer equal to the pointer addres returned
    char** no_spaces_str = tokenize(str, " ");
    printf("%s\n", str);

    for (int i = 0; i<numTokens(str, " "); i  ){
        printf("%s\n", no_spaces_str[i]);
    }
    //free heap allocated memory
    free(str);

    free(no_spaces_str);
    return 0;
}

results

tokenize-------------------------
buff[0]some
buff[1]literal
buff[2]string.
some
literal
string.
some