0

Currently I have the argc, argv and temp pieces placed in to be passed, and when I compile it returns no errors, but when I call the function later in the program and pass it a char array. It returns a stack dump. From what I have learned so far arrays cannot be passed back from a function and that is why I have passed the pointers.

int In2File(int argc, char *argv[], char *temp[] ){

    if (argc == 2) { //open file
        FILE *user_file;
        user_file = fopen(argv[1], "r");
        if (user_file == NULL) {
            printf("No data was found.\nEnd(Error 1)");
            exit(2);
        }
        else {
            int g = 0;//temp counter to load
            char c = 0;
            while ((c = fgetc(user_file)) != EOF && g <= tmplng - 1) { //Used Fgetc instead of fgets because Fgetc allows me to read
                *temp[g] = c;                      //until the end of the file and read each character. Even if there is an \n character.
                g++;                              // The second g < tmplng-1 is used to make sure that the \0 can be placed into the array.
            }
            printf("%s\n", temp);
            fclose(user_file);//Closed the txt file that was loaded by user in args(1)          
            printf("\nFile Loaded.\n");
        }
    }
    else if (argc > 2) { // Will exit if arguments are greater than 2.
        printf("Format: %s 'filename'", argv[0]);
        exit(1);
    }
    else {
        printf("File not provided. Enter information:\n");//If the user doesnt provide a file allow manual input.
        fgets(*temp, tmplng, stdin);
    }
}

In2File(argc,argv,temp);

Anyone have an idea as to where I went wrong with this function? I read a few similar posts but they were for C++ and Python. Which I havent learned C++ as yet and python is different to this beast called C.

Edit:

const int tmplng = 1000; //The only variables needed
    char temp[tmplng];       //
    char temp2[tmplng];      //

    printf("Starting....\n"); //Used for testing debugging. 
13
  • 1
    The code would be easier to follow if you remove all the unnecessary else statements which clutter it. For example after exit(2); there is no possible else. Commented Dec 6, 2016 at 20:20
  • 1
    Add more of your code as some things can't be understood clearly, like what is tmplng? Commented Dec 6, 2016 at 20:23
  • 2
    How is this function called, and how is the third argument declared? Commented Dec 6, 2016 at 20:23
  • There is an "if" a little further above used to check if the file returned NULL, if it does then it will exit. @WeatherVane Is that what you were talking about? Commented Dec 6, 2016 at 20:24
  • 2
    Note that const int tmplng = 1000; means that char temp[tmplng]; is a VLA (variable length array). C++ has different rules, but in C, it's a VLA. It doesn't hurt anything — you should just be aware. Commented Dec 6, 2016 at 20:27

3 Answers 3

3

The third parameter to the function doesn't match what the function is expecting. You're passing in a char [] (which decays to a char *), while the function expects char *[] (equivalently char ** as a function parameter).

The definition of the third parameter doesn't match how you intend to use it, which is as a character array.

Get rid of the extra level of indirection on the parameter, and adjust the function accordingly.

int In2File(int argc, char *argv[], char temp[] ){
        ...
        while ((c = fgetc(user_file)) != EOF && g <= tmplng - 1) { 
            temp[g] = c; 
            g++;  
        }
Sign up to request clarification or add additional context in comments.

2 Comments

Ok bud, I will do it now. Thanks you very much for your time and assistance. So just to clarify, when I pass a pointer of a pointer I use the double asterisk(**)?
@DamaniAPhilip Correct. In this case you just have a pointer.
0

This declaration of temp:

char temp[tmplng];

... does not go with this call ...

In2File(argc,argv,temp);

... to this function ...

int In2File(int argc, char *argv[], char *temp[] );

The function expects its third argument to be a pointer to a pointer to char *, but what it receives is a pointer to char. When it then tries to assign the character read ...

    *temp[g] = c;

That attempts to interpret the gth element of temp (itself interpreted as a char **) as a char *, to dereference that pointer, and to assign c to that location. That is highly unlikely to be a valid memory access.

It looks like you want to declare the third argument of In2File as either char *temp or char temp[], which are equivalent, and to write to it as temp[g] = c.

ALSO, as an aside, note that your

printf("%s\n", temp);

is problematic because you do not ensure that temp is null-terminated. And inasmuch as you support reading up to the very last byte, you don't have room to terminate it.

1 Comment

Thank you for this very detailed explanation, I also marked yours with the check, and I understand it now this helped me a lot. Thank you again
0

As numerous folks are pointing out in the comments, your code would be much easier to understand if you employed "early exit" for your error conditions.

It would also be a better separation of concerns to have main deal with the program arguments and let In2File just deal with a file pointer.

int main( int argc, char *argv[] ) {
    FILE *fp;

    if( argc == 2 ) {
        fp = fopen(argv[1], "r");
        if ( fp == NULL ) {
            fprintf(stderr, "Couldn't open %s: %s", argv[1], strerror(errno));
            exit(2);
        }
    }
    else if( argc > 2 ) {
        fprintf(stderr, "Usage: %s <optional filename>", argv[0]);
        exit(1);
    }
    else {
        fp = stdin;
    }

    char *lines = In2File(fp);

    printf("%s\n", lines);

    free(lines);
}

Note that stdin is a file pointer just like anything you opened with fopen. I've also used strerror and errno to provide the reason the file didn't open.

Now In2File just takes a file pointer. Also note that I have it returning the content, not passing in a preallocated array. This is because when reading from input you never know how much input you're going to get. You either must stop reading before you run out of space, or allocate more memory. You can't reallocate stack memory, so it's best to let In2File control allocating memory. It also avoids having to pass around a global variable.

Once that's done, In2File becomes much simpler.

static char *In2File( FILE *fp ){
    size_t read_size = 1024;

    /* Don't forget space for null */
    char *content = calloc( read_size + 1, sizeof(char) );

    fread( content, sizeof(char), read_size, fp );

    return content;
}

Rather than step through a character at a time, I've used fread to read a block from the file not larger than the amount I've allocated. Memory reallocation is a topic for another time.

1 Comment

Thank you for this, I am going to research those new functions that you put in like stderr, errno, and calloc. I have not learned these as yet, and it looks like I got in over my head at the moment in trying to make this function. lol I have a lot more reading to do, back to the book(s).

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.