OSDev.org

Posted: **Wed Feb 04, 2009 3:21 pm**

I doubt this isn't anything you guys haven't seen before, but I've been sluggishly tinkering with my tiny kernel and its C code. I tried to add a simple echo command to the command line, but it seems that every time I issue an echo command, the kernel pagefaults.

Why this is happening I don't understand. I'm betting it's something to do with my C code, if anything. Seeing as it's a kernel that's doing the pagefaulting, but most likely resulting from bad code, I figured asking here would be a good idea.

Here is my command interpreter function command_interpret(). The first parameter is contains whatever the user has typed in and is called from the keyboard driver.

Code: Select all


char echocmd[] = "echo ";

...
// here we have a few other commands as well as the echocmd variable
// and a few functions to be called when some commands are executed
// pay no mind to this area, it's unimportant to the issue at hand
...

int command_interpret(char *buffer) {
	// debug code, comment out if you don't need it
	//putln(buffer);
	kbdisable();
	if (strcmp(buffer,hello) == 0) {
		putln("Hello World!");
	} else if (strcmp(buffer,ver) == 0) {
		putln("Emerald kernel-0.1.5 (generic)");				
	} else if (strcmp(buffer,halt) == 0) {
		puts("System halted, it is now safe to turn off the computer.");
		for(;;);				
	} else if (strcmp(buffer,uptimecmd) == 0) {
		getuptime();			
	} else {
		// Here, we have the invalid command error, and we also have
		// space to check for commands that require substrings.
		if (strncmp(buffer,echocmd,5) == 0) {
			putln(strtrun(buffer,strlen(echocmd)));	 ///////////////////////////////////////  I believe this is the problematic line. //////////////////
		} else {
			putln("Invalid command."); // I'll probably change this later. Let's get filesystem support in first.			
		}
	}
	putln(" ");
	prompt();
	kbenable();
	return 0;
}

putln is a command that simply calls puts() twice: the first time to output whatever you put in putln()'s parentheses as the first parameter, the second time to simply puts("\n"); in order to reduce the amount of code. (Imagine having to have two instances of puts() for every putln(). That would suck.)

You're probably wondering what strtrun() does. Basically it returns the value of the first parameter NOT INCLUDING the first few characters (defined by the second parameter). strtrun means String Truncate. Here's the code for it:

Code: Select all

char *strtrun(char *trn, int s) {
	int l;
	char *tr;	
	for(l=0; l < s; l++) {
		tr[l] = trn[l];	
	}
	return tr;
}

And for the sake of completeness, I have included my strlen() function.

Code: Select all

size_t strlen(const char *str)
{
	size_t retval;
	for(retval = 0; *str != '\0'; str++) retval++;
	return retval;
}

Surely I made a mistake in my C code that could have been avoided.

Exactly what did I do wrong here, and why would it be page faulting? And before you ask, NO, I do not have a proper stack-trace function in my panic() to show where the pagefault occured. I probably should, but at this point I can't provide the information, and as stated earlier I'm fairly certain that it was a mistake in my C code.

Any ideas on what's triggering the pagefault?

Posted: **Wed Feb 04, 2009 3:29 pm**

char *tr; <- uninitialized
tr[l] = trn[l]; <- tr points nowhere, pagefault follows

Posted: **Wed Feb 04, 2009 4:01 pm**

That means I will probably need to simply allocate memory to it, IIRC.

To wit:

char *tr = "";

^ I assume that would work, but there's only one way to find out.

Either that, or I'd have to malloc() somewhere. I don't typically use malloc(), so I'm a bit rusty on where to put it in the code at this point.

Posted: **Wed Feb 04, 2009 5:09 pm**

No, that won't work. By writing

Code: Select all

char* tr = "";

you are allocating only a single byte (i.e. a zero terminator) on the stack. In addition, when the function returns, any local variables are not valid anymore so the calling function would crash if it used the return value.

You have two options:

Code: Select all

char* tr = (char*)malloc(s + 1);

or

Code: Select all

static char tr[MAX_STRING_LEN];

Using the first option you are allocating enough memory for the copied string plus a zero terminator from the heap. The caller should 'free' the memory when finished with it.

For the second option you need to #define MAX_STRING_LEN to be a value that is guaranteed to be larger than the largest value of 's+1' passed to your function. You should add a check before the copy to ensure this. The variable is declared as static so it is not allocated on the stack and can safely be returned to the caller.

Another thing, you need to zero terminate the copied string. Add the line

Code: Select all

tr[l] = '\0';

after the 'for' loop.

The function copies the first part of the command string. Is that your intention? For example, if you have the command:

Code: Select all

echo Hello World

and echocmd="echo " then the function would print "echo " and not the part after the command.

Posted: **Wed Feb 04, 2009 7:18 pm**

Something tells me you're not sure how C works and what goes on in memory. Re-read Combuster's post more carefully. That's not what he meant.

Posted: **Wed Feb 04, 2009 7:21 pm**

I went ahead and tweaked my for() loop.

Here's my new code for strtrun():

Code: Select all

char *strtrun(char *trn, int s) {
   int l;
   char* tr = (char *)malloc(s + 1);
   for(l=s; l < strlen(trn); l++) {
      tr[l] = trn[l];
   }
   tr[1] = '\0';
   return tr;
}

But now, typing the echo command followed by a string of text results in a wonderful heap-and-stack collision. Here's a screenshot. http://img17.imageshack.us/img17/7428/e ... ailnz4.png

I've never dealt with one of these before, in all of my years of programming. Exactly what is going on?

Posted: **Wed Feb 04, 2009 7:48 pm**

Code: Select all

char* strtrun(char* trn,int s)
{
      int len = strlen(trn),l;
      char* tr = (char*) malloc(len-s+1);
      for(l = s; l <= len; l++) tr[l-s] = trn[l];
      tr[s+1] = '\0';
      return tr;
}

EDIT: he claims on IRC that he still has problems but it's probably unrelated with this function (which is supposed to strip the first s characters) - (tested, btw).

OSDev.org

A strange page fault.

A strange page fault.

Re: A strange page fault.

Re: A strange page fault.

Re: A strange page fault.

Re: A strange page fault.

Re: A strange page fault.

Re: A strange page fault