1

So I have a function called scanCode which scans words from a text file and stores it in a 2D array. I then want to return this array into an array variable in the main function, this is my code so far

#include <stdio.h>

char **scanCode()
{
    FILE *in_file;
    int i = 0;
    static char scan[9054][6];

    in_file = fopen("message.txt", "r");
    while (!feof(in_file))
    {

        fscanf(in_file, "%s", scan[i]);
        i++;
    }
    return scan;
}

int main(void)
{

    int hi[9053];

    FILE *in_file;

    in_file = fopen("message.txt", "r");

    char **array = scanCode();

    printf("%c", array[0]);
    printf("%c", array[1]);
    printf("%c", array[2]);
    printf("%c", array[3]);
}

So basically the array returned from the scanCode function I want it to be stored in the char array in the main function.. after looking at a lot of questions and answers here, this is what I got to but the pointer etc is hard to understand for me.. could someone tell me what I did wrong here?

Improve this question
10
  • Why are you using %c to print a string, array[n]? Commented Dec 4, 2015 at 17:27
  • well when i put %s, the cmd run window freezes so there is sometihng wrong with the array Commented Dec 4, 2015 at 17:31
  • Using the wrong format specifier isn't going to fix the problem with the array. ;) Do you have any strings in your message.txt file that are longer than 5 characters? And why did you open the file twice (once in main and once in the function)? That's a bad idea. Commented Dec 4, 2015 at 17:33
  • 1
    One other point: while (!feof(in_file)) is a bad way to read until end of file. feof will only be true on an attempt to read beyond end of file, so your code will attempt an additional fscanf after the last line of the file has already been read, and you'll have one bogus entry at the end of your scan array. See Why is “while ( !feof (file) )” always wrong?. Commented Dec 4, 2015 at 17:46
  • 1
    @lurker OP has had 3 replies in previous answers to not use while (feof() ... Perhaps this 4th one will do the trick. Commented Dec 4, 2015 at 17:56

3 Answers 3

Reset to default
4

Change the return type of the function the following way

#include <stdio.h>

char ( *scanCode() )[6]
{
    FILE *in_file;
    int i = 0;
    static char scan[9054][6];

    in_file = fopen("message.txt", "r");
    while (!feof(in_file))
    {

        fscanf(in_file, "%s", scan[i]);
        i++;
    }
    return scan;
}

int main(void)
{

    int hi[9053];

    FILE *in_file;

    in_file = fopen("message.txt", "r");

    char ( *array )[6] = scanCode();

    printf("%s", array[0]);
    printf("%s", array[1]);
    printf("%s", array[2]);
    printf("%s", array[3]);
}

Also in the printf statements use format specifier %s

And change the loop in the function like

    while ( i < 9054 && fscanf(in_file, "%5s", scan[i]) == 1 ) ++i;
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

What happens to scan[][] when you get out of scope?
@Bob__ I think nothing. Do you have another opinion?
@Bob__ since it's declared static, it's contents are available outside of the scope of the scanCode function.
I missed the static, my bad.
2

I prefer to simplify the code in this way:

#include <stdio.h>

#define NumLines   9054
#define NumCols    6

void freeMem(char **ele) {
    while (*ele != NULL) {
        free(*ele);
        ele++;
    }
}

char **scanCode(char *fileName)
{
    FILE *in_file;
    char readingFormat[128];
    int i = 0;

    /*
     * Instead to declare a static variable I prefer to allocate dynamically
     * the bidimensional array.
     * It is done in two steps:
     * 1. allocate the memory for the first dimension
     * 2. for each element in this dimension allocate the memory for each element in the second dimension
     *
    */
    char **scan = (char **)malloc((NumLines + 1) * sizeof(char *));
    if (scan == NULL) {
        return NULL;
    }
    for (int j = 0; j < NumLines; j++) {
        scan[j] = (char *)malloc(NumCols + 1);
        if (scan[j] == NULL) {
            freeMem(scan);
            return NULL;
        }
        scan[j][0] = NULL;
    }
    scan[NumLines] = NULL;  // define the end of memory

    in_file = fopen(fileName, "r");
    if (fopen == NULL) {
        freeMem(scan);
        return NULL;
    }
    sprintf(readingFormat, "%%%ds", NumCols);
    while (fscanf(in_file, readingFormat, scan[i]) == 1 && i < NumLines) {
        i++;
    }
    return scan;
}

int main(void)
{
    char **array = scanCode("message.txt");
    if (array == NULL) {
        printf("ERROR\n");
        exit(0);
    }
    for (char **tp = array; **tp != NULL; tp++) {
        printf("%s\n", *tp);
    }
}
Improve this answer

7 Comments

I personally prefer to pass the file name as a parameter. You should get rid of the magic numbers (9054?) and check directly the scanf() not the foef().
This is a sensible approach (+1), although I would probably just use malloc(6) in place of malloc(6 * sizeof(char)) since sizeof(char) is well-understood (this is the only exception I generally make to explicitly indicating a sizeof). Also, I know in this case since main just exits, it's worth noting that the caller to scanCode() needs to be responsible for freeing the memory for the return pointer once it's no longer needed. I also agree with the improvements suggested by @Bob__.
fscanf(in_file, "%s", scan[i]); is just as safe as gets(scan[i]). Consider adding a limitation on input length.
I certainly don't think this is the best idea. You should avoid using 'malloc' as much as you can.
@FISOCPP why should malloc be avoided as much as one can?
|
1

Arrays aren't pointers (hello from me again).

This:

static char scan[9054][6];

have the most obvious type you would expect it to be - 'char [9054][6]' and not 'char **'. It's spelled as an array of 6 elements each of which is another array of 9054 chars. On the other hand the type 'char **' is spelled as 'a pointer to pointer to char' and as you can probably see now they are entirely different things.

Your code should look something like this:

#include <stdio.h>

typedef char yourArrayType[9054][6];

typedef struct { yourArrayType return_value; } letsReturnArraysType;

letsReturnArraysType scanCode()
{
    FILE *in_file;
    int i = 0;
    yourArrayType scan;

    in_file = fopen("message.txt", "r");
    while (!feof(in_file))
    {

        fscanf(in_file, "%s", scan[i]);
        i++;
    }
    return *(letsReturnArraysType*)scan;
}

int main(void)
{

    int hi[9053];

    FILE *in_file;

    in_file = fopen("message.txt", "r");

    letsReturnArraysType arrayStruct = scanCode();

    printf("%s", arrayStruct.return_value[0]);
    printf("%s", arrayStruct.return_value[1]);
    printf("%s", arrayStruct.return_value[2]);
    printf("%s", arrayStruct.return_value[3]);
}
Improve this answer

3 Comments

No as I'm returning the whole array.
I see. But not very efficient for a large array. Internally, the compiler generates a memcpy to make a whole new copy of the array for the return.
Well yeah you're right. But my point was rather educational then efficient. About your last statement - yeah if your compiler is crap but most recent compilers will certainly optimize it. I guess they'll treat my local 'scan' as directly the return-value itself or maybe even the local in main 'arrayStruct'.

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.