0

I want to change my input.txt file to an integer array. But sadly I keep missing one integer whenever new-line-character is met.

Following is my main()

int main(int args, char* argv[]) {
  int *val;

  char *STRING = readFile();

  val = convert(STRING);

  return 0;
}

Following is my file input function

char *readFile() {
    int count;
    FILE *fp;

    fp = fopen("input.txt", "r");
    if(fp==NULL) printf("File is NULL!n");

    char* STRING;
    char oneLine[255];
    STRING = (char*)malloc(255);
    assert(STRING!=NULL);

    while(1){
        fgets(oneLine, 255, fp);
        count += strlen(oneLine);
        STRING = (char*)realloc(STRING, count+1);
        strcat(STRING, oneLine);
        if(feof(fp)) break;
    }
    fclose(fp);
    return STRING;
}

Following is my integer array function

int *convert(char *STRING){
    int *intarr;
    intarr = (int*)malloc(sizeof(int)*16);
    int a=0;
    char *ptr = strtok(STRING, " ");


    while (ptr != NULL){
        intarr[a] = atoi(ptr);

        printf("number = %s\tindex = %d\n", ptr, a);
        a++;
        ptr = strtok(NULL, " ");
    }

    return intarr;
}
6
  • 3
    Your feof logic is broken. In fact, you should never use feof. Commented Oct 5, 2018 at 12:31
  • 1
    Calling realloc for each line is terribly inefficient. And all caps symbols (STRING) shouldn't be used, they are usually used for macros. Also consider this line if (fp == NULL) printf("File is NULL!n"); what do you think happens if fp is NULL? Commented Oct 5, 2018 at 12:35
  • 1
    strcat(STRING, oneLine); concatenating a string works better if you start with an empty string or with some other defined start value. Your STRING points to memory with "random" content. Commented Oct 5, 2018 at 12:41
  • From what is shown here, the string buffer itself is pointless. Just scanning white space separated integers out of the original file and properly managing a dynamic-growing int array while doing so (your current code uses an unchecked fixed size fo 16, which is terrible), would be sufficient. Commented Oct 5, 2018 at 12:51
  • 1
    Please edit your question and show a sample of you input file. Commented Oct 5, 2018 at 13:58

1 Answer 1

1

There are many issues.

This is a corrected version of your program, all comments are mine. Minimal error checking is done for brevity. intarr = malloc(sizeof(int) * 16); will be a problem if there are more than 16 numbers in the file, this should be handled somehow, for example by growing intarr with realloc, similar to what you're doing in readFile.

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

char *readFile() {
  FILE *fp;

  fp = fopen("input.txt", "r");
  if (fp == NULL)
  {
    printf("File is NULL!n");
    return NULL;      // abort if file could not be opened
  }

#define MAXLINELENGTH 255      // define a constant rather than hardcoding "255" at several places

  char* STRING;
  char oneLine[MAXLINELENGTH];
  STRING = malloc(MAXLINELENGTH);
  int count = MAXLINELENGTH;   // count mus be initialized and better declare it here
  assert(STRING != NULL);
  STRING[0] = 0;          // memory pointed by STRING must be initialized

  while (fgets(oneLine, MAXLINELENGTH, fp) != NULL)   // correct usage of fgets
  {
    count += strlen(oneLine);
    STRING = realloc(STRING, count + 1);
    strcat(STRING, oneLine);
  }

  fclose(fp);
  return STRING;
}


int *convert(char *STRING, int *nbofvalues) {   // nbofvalues for returning the number of values
  int *intarr;
  intarr = malloc(sizeof(int) * 16);
  int a = 0;
  char *ptr = strtok(STRING, " \n");   // strings may be separated by '\n', or ' '

  *nbofvalues = 0;

  while (ptr != NULL) {
    intarr[a] = atoi(ptr);

    printf("number = %s\tindex = %d\n", ptr, a);
    a++;
    ptr = strtok(NULL, " \n");  // strings are separated by '\n' or ' '
   }                            // read the fgets documentation which
                                // terminates read strings by \n

  *nbofvalues = a;    // return number of values 
  return intarr;
}


int main(int args, char* argv[]) {
  int *val;

  char *STRING = readFile();

  if (STRING == NULL)
  {
    printf("readFile() problem\n");   // abort if file could not be read
    return 1;
  }

  int nbvalues;
  val = convert(STRING, &nbvalues);  // nbvalues contains the number of values

  // print numbers
  for (int i = 0; i < nbvalues; i++)
  {
    printf("%d: %d\n", i, val[i]);
  }

  free(val);    // free memory
  free(STRING); // free memory
  return 0;
}

I'm not sure what your requirement is, but this can be simplified a lot because there is no need to read the file into memory and then convert the strings into number. You could convert the numbers on the fly as you read them. And as already mentioned in a comment, calling realloc for each line is inefficient. There is room for more improvements.

Sign up to request clarification or add additional context in comments.

1 Comment

Don't cast the result of *alloc()!

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.