0

I'm trying to build alphanumeric strings consisting of 3 initial letters and 3 final numbers and save them in a .txt file. I wrote this:

int i = 0,          
    j = 0;     
char name_cpu[8],
     array_numbers_final[8],    
     array_letters[27] = "ABCDEFGHIJKLMNOPQRSTUWXYVZ",
     array_numbers[10] = "123456789";


/* Generator data */
  for(i = 0; i < number_cpu; i++)
  { 
      for(j = 0; j < 3; j++){
      name_cpu[j] = array_letters[rand() % (sizeof(array_letters)-1)];
      array_numbers_final[j] = array_numbers[rand() % (sizeof(array_numbers)-1)];
    }

    strcat(name_cpu, array_numbers_final);
    fprintf(list_cpu, "%s \n", name_cpu);
  }

The problem is that at the first external for loop it correctly prints a string of the form "AAA000". At the second for loop it goes in segmentation fault. Can anyone tell me what and where I am doing wrong?

EDIT: A minimal reproducible example is:

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

int main (void){

  FILE *list_cpu = NULL;
  int i = 0,
  number_cpu = 3,    
  j = 0;     
  char name_cpu[8] = {0},
  array_numbers_final[8] = {0},    
  array_letters[27] = "ABCDEFGHIJKLMNOPQRSTUWXYVZ",
  array_numbers[10] = "123456789";
        
  list_cpu = fopen("list_cpu.txt", "w");  
        
        
  /* Generator data */
  for(i = 0; i < number_cpu; i++)
  { 
    for(j = 0; j < 3; j++){
      name_cpu[j] = array_letters[rand() % (sizeof(array_letters)-1)];
      array_numbers_final[j] = array_numbers[rand() % (sizeof(array_numbers)-1)];
  }
   strcat(name_cpu, array_numbers_final);
   fprintf(list_cpu, "%s \n", name_cpu);
  }
  fclose(list_cpu);

return(0);
}

If number_cpu is equal to 1 it works. But if it is higher then 1, the program goes into segmentation fault.

5
  • 1
    You can always use a debugger to find where exactly the segmentation fault occurs. Commented Dec 16, 2020 at 21:50
  • 2
    you call strcat on something which is not a null-terminated string Commented Dec 16, 2020 at 21:52
  • Please extract a minimal reproducible example and provide that as part of your question. As a new user, also take the tour and read How to Ask. Commented Dec 16, 2020 at 22:52
  • @UlrichEckhardt sorry about that. I edited the post as you said Commented Dec 17, 2020 at 10:38
  • regarding: char name_cpu[8] = {0}; and name_cpu[j] = array_letters[rand() % (sizeof(array_letters)-1)]; 'name_cpu[]` is 8 characters, however, the posted code is trying to insert a binary value, with range -1...26 into each of the first 3 characters and will do this 3 times, into the same first 3 characters. If your looking for some 'random' values, just do it once and the first thing in main() should be initializing the random number generator with something like: #include <time.h> and srand( (unsigned)time( NULL ) ); Commented Dec 18, 2020 at 17:48

1 Answer 1

1

answer to new edits to OP

Edits to OP include initialization of variables addressing some instances of undefined behavior in original code, however buffer overflow problem remains in following section:

/* Generator data */
  for(i = 0; i < number_cpu; i++)
  { 
    for(j = 0; j < 3; j++){
      name_cpu[j] = array_letters[rand() % (sizeof(array_letters)-1)];
      array_numbers_final[j] = array_numbers[rand() % (sizeof(array_numbers)-1)];
  }
   strcat(name_cpu, array_numbers_final);
   fprintf(list_cpu, "%s \n", name_cpu);
  }
  fclose(list_cpu);

Where name_cpu is defined with 8 char: char name_cpu[8] = {0}

"If number_cpu is equal to 1 it works. But if it is higher then 1, the program goes into segmentation fault."...

After first iteration of outer loop, including one strcat of name_cpu and list_cpu, the char name_cpu is populated with 6 new characters and \0 in remaining locations. Eg:

//example
|S|J|P|Y|E|P|\0|\0|  //

The error occurs at end of 2nd iteration of outer loop at the strcat() statement. The inner loop re-populated the first three positions of name_cpu, eg:

//first three positions changed, the rest remains
|C|B|A|Y|E|P|\0|\0|  // 
 ^ ^ ^ updated values

Error occurs when the new value for array_numbers_final :

|G|H|O|\0|\0|\0|\0|\0|

is attempted to be concatenated to name_cpu, resulting in:

|C|B|A|Y|E|P|G|H|O|  //attempt to write to memory not owned
                ^end of buffer

Resulting in buffer overflow error, and likely the segmentation fault error condition. Again, as before, design variables to meet the potential needs of of the program. In this instance the following will work:

 char name_cpu[100] = {0};
 char array_numbers_final[100] = {0};     

answer to original post:
The problem is here:

strcat(name_cpu, array_numbers_final);//undefined behavior.

First, name_cpu[8] is uninitialized at time of declaration, nor is it initialized at any time before used. Because it is not guaranteed what the contents are, it is not necessarily a string. Using this variable as is in any string function can invoke undefined behavior

Furthermore,(even if name_cpu is a valid string) when array_numbers_final is concatenated to name_cpu, the buffer will overflow. During my test run I saw this:

enter image description here

The fix is to start off with initialized buffers of sufficient size for intended purpose. eg:

 char name_cpu[100] = {0}
 array_numbers_final[100] = {0},    
 array_letters[27] = "ABCDEFGHIJKLMNOPQRSTUWXYVZ",
 array_numbers[10] = "123456789";

Code example below is adaptation using your provided example, with suggested edits. Read in-line comments:

int main(void)
{
    int number_cpu = 3;//added - previously not defined in OP
    int i = 0,          
    j = 0;     
    char name_cpu[100] = {0},//resized and initialize
     array_numbers_final[100] = {0},//resized and initialize    
     array_letters[27] = "ABCDEFGHIJKLMNOPQRSTUWXYVZ",
     array_numbers[10] = "123456789";


/* Generator data */
      for(i = 0; i < number_cpu; i++)
      { 
          for(j = 0; j < 3; j++){
          name_cpu[j] = array_letters[rand() % (sizeof(array_letters)-1)];
          array_numbers_final[j] = array_numbers[rand() % (sizeof(array_numbers)-1)];
        }

        strcat(name_cpu, array_numbers_final);
        fprintf(stdout, "%s \n", name_cpu);//to stdout
        //        ^ changed here
      }
    return 0;
}
Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.