Search code examples
cfilefputs

Why my fputs doesn't work when the file already contains anything?


I try to add a new line to the already existing file, which already consists some text. I found out that when the file contains something, the program stops working after fputs(stringToAdd, myFile); in function void Save(struct SEED arrayToSave[], int size). However, when I try to write in an empty file, everything works properly. Can you guys please help me?

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

int numberOfItems = 0;
char path[] = "Catalogue.txt";
char text[1000000];

struct SEED {
  int index;
  char type[30];
  char name[60];
  float mass;
  float price;
};

void GetText() {
  FILE* catalogueFile;
  char ch; // temporary char
  int counter = 0;

  catalogueFile = fopen(path, "r");
  while((ch = fgetc(catalogueFile)) != EOF ) {
    text[counter] = ch;
    counter++;
  }
  fclose(catalogueFile);
}

void CalculateNumberOfItems() {
  for(int i = 0; i < strlen(text); i++) {
    if(text[i] == '\n') numberOfItems++;
  }
}

struct SEED* GetArrayOfStructs() {
  GetText();
  CalculateNumberOfItems();
  FILE* catalogueFile;
  int counter = 0;
  char* arrayOfSubstrings[numberOfItems];
  struct SEED* arrayOfStructs = malloc(numberOfItems * sizeof(arrayOfStructs));
  struct SEED* arrayOfStructsPointer = arrayOfStructs;

  char* token = strtok(text, "\n");
  while( token != NULL ) {
      arrayOfSubstrings[counter] = token;
      token = strtok(NULL, "\n");
      counter++;
   }

   for(int i = 0; i < numberOfItems; i++) {
     struct SEED tempStruct;
     sscanf(arrayOfSubstrings[i], "%d %s %s %f %f", &tempStruct.index, tempStruct.type, tempStruct.name, &tempStruct.mass, &tempStruct.price);
     arrayOfStructs[i] = tempStruct;
   }

   return arrayOfStructsPointer;
}

void Save(struct SEED arrayToSave[], int size) {
  FILE* myFile = fopen("Catalogue.txt", "w");
  char fullString[1000000];

  for(int i = 0; i < size; i++) {
    struct SEED currentStruct = arrayToSave[i];
    char stringToAdd[1000];
    sprintf(stringToAdd, "%d %s %s %f %f\n", currentStruct.index, currentStruct.type, currentStruct.name, currentStruct.mass, currentStruct.price);
    strcat(fullString, stringToAdd);
  }
  printf("%s\n", fullString);
  fputs(fullString, myFile);
  fclose(myFile);
}

void AddNew() {
  struct SEED* oldArrayOfStructs = GetArrayOfStructs(path);
  struct SEED newArrayOfStructs[numberOfItems + 1];
  struct SEED newStruct;
  int newIndex = numberOfItems + 1;
  char newType[30], newName[60];
  float newMass, newPrice;
  char tempChar[200];

  printf("Input type, name, mass and price\nof the new seed whith spaces: ");
  fgets(tempChar, 200, stdin);
  fgets(tempChar, 200, stdin);

  sscanf(tempChar, "%s %s %f %f\\n", newType, newName, &newMass, &newPrice);

  newStruct.index = newIndex;
  strcpy(newStruct.type, newType);
  strcpy(newStruct.name, newName);
  newStruct.mass = newMass;
  newStruct.price = newPrice;

  printf("%d\n", numberOfItems);

  for(int i = 0; i < numberOfItems; i++) {
    newArrayOfStructs[i] = oldArrayOfStructs[i];
  }

  newArrayOfStructs[numberOfItems] = newStruct;

  Save(newArrayOfStructs, numberOfItems + 1);
  printf("\n");
}

void UserInput() {
  char choice;
  int choiceInt, a = 1;

  printf("Add new item - print 1\n");
  printf("\n");

  printf("Input your choice: ");
  choice = getchar();
  choiceInt = choice - '0';
  printf("\n");

  switch(choiceInt) {
    case 1:
      AddNew();
      break;
    default:
      printf("Bye!\n");
      break;
  }
}

int main() {
  UserInput();
  return 0;
}

Solution

  • At least these problems:

    Allocation too small

    Code incorrectly allocates for an array of pointers.

    // struct SEED* arrayOfStructs = malloc(numberOfItems * sizeof(arrayOfStructs));
    struct SEED* arrayOfStructs = malloc(numberOfItems * sizeof arrayOfStructs[0]);
    

    char too small to distinguish 257 different responses from fgetc()

    // char ch; // temporary char
    int ch;
    ...
    while((ch = fgetc(catalogueFile)) != EOF ) {
    

    Potential buffer overrun

    Use a width.

    No point scanning for 2 characters \ and n with "\\n".

     // sscanf(tempChar, "%s %s %f %f\\n", newType, newName, &newMass, &newPrice);
     sscanf(tempChar, "%29s %59s %f %f", newType, newName, &newMass, &newPrice);
    

    Best to test sscanf() result too.

    if (sscanf(tempChar, "%29s %59s %f %f", newType, newName, &newMass, &newPrice) != 4) {
      TBD_error_code();
    }
    

    Overflow risk

    // while((ch = fgetc(catalogueFile)) != EOF ) {
    while(counter < (sizeof text - 1) && (ch = fgetc(catalogueFile)) != EOF ) {
      text[counter] = ch;
    

    Maybe overflow risk

    // sprintf(stringToAdd, "%d %s %s %f %f\n",
    snprintf(stringToAdd, sizeof stringToAdd, "%d %s %s %f %f\n",