Having trouble with C string and File operations

[mothergoose729]

I have a project for my systems programming class in which I have to create the history function for bash shell. I am trying to write out the input from stdin into a file and the read the contents of that file to the screen when the user types "history". I am getting some pretty strange errors. Namely the input to my file is not right, some of the commands that should only be in their once are often in their more than once. Also, when I try and type history I get a segmentation fault.

I am only allowed to use C functions for the strings (no c++). I wasn't very good at string manipulation to begin with, so I am having a hard time keeping track of everything.

The relevant parts of the code are at the bottom. It doesn't matter what parser does; this was implemented by my instructor and how it works is not really part of the assignment.

#include "parser.h" //header file that implements shell features
#include "shell.h" //header file that implements shell features
#include <string.h>
#include <stdio.h>

const char* HISFILE = "simpleshell_history"; //name of history file

int main(void) {
  char input[MAXINPUTLINE] = "\0";
  char* command = NULL; 
  FILE* history = NULL;
  fpos_t pos; //this is the struct returned by fgetpos(). I am trying to use it to save the file state so I can do the history read, and then let it write again from the same position
  signal_c_init();
 
  history = fopen(HISFILE, "ab+"); //is this the write input mode?
    

//some screen output stuff
  printf("Welcome to the sample shell!  You may enter commands here, one\n");
  printf("per line.  When you're finished, press Ctrl+D on a line by\n");
  printf("itself.  I understand basic commands and arguments separated by\n");
  printf("spaces, redirection with < and >, up to two commands joined\n");
  printf("by a pipe, tilde expansion, and background commands with &.\n\n");
  
  printf("\nktsh$ ");

  while (fgets(input, sizeof(input), stdin))//input is the command from keyboard 
{
  fputs(input, history); //trying to write this to a file exactly, with the new line
   
   if(strncmp(input, "history\n", sizeof("history\n")) == 0)//compares input to "history" using strncmp. 
  {
	fgetpos(history, &pos); //this returns a file pointer structure. Using this to save the position of the file when it was writing.
	rewind(history); //resets file pointer to beginning of the file
	while (feof(history) == 0) //checks for end of file
	{
		int lineCount = 1;
		fgets(command, sizeof(input), history);
		printf("%d",lineCount);
		printf(" - %s",command); //some output. Trying to get "1 - (command)
                lineCount++;
	}
    	fsetpos(history, &pos); //trying to return file pointer to initial position
  }
    stripcrlf	
    parse(input);
    printf("\nktsh$ ");
  }

  fclose(history);
  return 0;
}

Here is the contents of my history file after I type "CD, LS, echo stuff".

//this space is intentional
cd
cd
ls
echo stuff
history

Any ideas where I went wrong? Just some general explanation about what is going on here and why this is wrong would be very helpful. Thank you very much for any help.

[Chervil]

I'm more familiar with C++ rather than C. But still, this looks wrong:

    char* command = NULL; 


    fgets(command, sizeof(input), history);

command is a null pointer, so trying to use it doesn't make sense.

Sorry, that's as far as I got.

[mothergoose729]

Hey thanks for the reply.

fgets is supposed to change the character pointer passed to it to point a character string extracted from the file. I was using fgets with command unitialized and my compiler was giving me a warning, so I set it to null so it would be quite. The return type for fgets is actually an integer... all of the standard C functions for file operations are pretty weird like that, I am finding out.

Anybody else have some ideas? Still stuck...

[MikeyBoy]

fgets may change the contents of the memory that command points to, but it can't possible change the value of command itself. In C, everything passed by value - there are no references.

[mothergoose729]

Ok now I am getting pretty confused.

Here is the reference link to fgets from cplusplus:

http://www.cplusplus.com/reference/cstdio/fgets/

If I am interpreting it correctly, the char* passed to fgets changed so that it points to the character string attached to the file. On success, fgets returns the character string, and on failure it returns a null pointer.

I changed the following bits of my code:

char * command; //from char* command = NULL;
.
.
.
while (feof(history) == 0) //checks for end of file
	{
		int lineCount = 1;
		command = fgets(command, sizeof(input), history); //former fgets(command, size(input), history);
                if (command != NULL) //added error check
               {
		          printf("%d",lineCount);
		         printf(" - %s",command); //some output. Trying to get "1 - (command)
                lineCount++;
              }

	}
    	fsetpos(history, &pos); //trying to return file pointer to initial position
  }

I am still having all the same problems as before.

[Chervil]

Did you consider using this:

    char command[MAXINPUTLINE]; 

it may not fix all of the problems, but at least will allow the fgets() function to operate as intended.
http://www.cplusplus.com/reference/cstdio/fgets/

Look at the example in the link, or even at the example in your own code,

char input[MAXINPUTLINE] = "\0";

fgets(input, sizeof(input), stdin)

Notice that input doesn't get changed so that it points to something else. Rather, the contents of the memory pointed to by input is changed.

[MikeyBoy]

Ok now I am getting pretty confused.

Here is the reference link to fgets from cplusplus:

http://www.cplusplus.com/reference/cstdio/fgets/

That says nothing that disagrees with what I said.

If I am interpreting it correctly, the char* passed to fgets changed so that it points to the character string attached to the file.

No. It says that the characters that get read from the file are stored to the location pointed to by the char*. Now, what do you think will happen if that location is NULL?

You do understand what a pointer is, right?

And you do understand that all function arguments in C are passed by value, not by reference, so the value of the aregument in the calling function does not change?

[mothergoose729]

Thanks chervil that worked wonderfully.

[mothergoose729]

Alright, my code is working 95% as it should now. I have a minor problem where when I type history, the last line of the history file is printed twice. It doesn't appear to be detecting of of file soon enough.. going to ask my instructor about this.

The duplicate entries in my file went away when I started opening and closing the file with every input operation. Not sure why this solved the problem but I am happy its solved.

My updated code

#include "parser.h"
#include "shell.h"
#include <string.h>
#include <stdio.h>

const char* HISFILE = "simpleshell_history";

int main(void) {
  char input[MAXINPUTLINE] = "\0";
  char command[MAXINPUTLINE]; 
  FILE* history = NULL;
  fpos_t pos; 
  signal_c_init();
 
  printf("Welcome to the sample shell!  You may enter commands here, one\n");
  printf("per line.  When you're finished, press Ctrl+D on a line by\n");
  printf("itself.  I understand basic commands and arguments separated by\n");
  printf("spaces, redirection with < and >, up to two commands joined\n");
  printf("by a pipe, tilde expansion, and background commands with &.\n\n");
  
  printf("\nktsh$ ");

  while (fgets(input, sizeof(input), stdin)) {
  history = fopen(HISFILE, "a+");
  fputs(input, history);
  fclose(history); 
   if(strncmp(input, "history\n", sizeof("history\n")) == 0)
  {
	history = fopen(HISFILE, "r");
    int lineCount = 1;
	do
	{
		if (fgets(command, sizeof(command), history));
		{
			printf("%d",lineCount);
			printf(" - %s",command);
			lineCount++;
		}
	} while (feof(history) == 0);
		fclose(history);
  }
    stripcrlf(input);	
    parse(input);
    printf("\nktsh$ ");
  }

  fclose(history);
  return 0;
}

[Chervil]

Testing for feof here is not the best way to handle reading from the file. Try this:

    history = fopen(HISFILE, "r");
    int lineCount = 1;

    while (fgets(command, sizeof(command), history))
    {
        printf("%d",lineCount);
        printf(" - %s",command);
        lineCount++;
    }

    fclose(history);

[mothergoose729]

I figured out the problem, I had a dangling semicolon on the fgets() check. I guess I didn't need to check for feof() at all, tried what you did chervil and it works just fine. Thanks again for the help. Marked as solved.