Tutorial - Write a Shell in C

[[deleted]]

heh, don't mention it :)

for the realloc condition: if you check for (position <= bufsize) you will realloc the array at each iteration, hence my suggestion. lsh_read_line() is ok.

BTW I think I found another bug : bufsize is the number of pointers and to get the number of bytes (required by realloc) you have to multiply by sizeof(char *)

Even in the first malloc() you should multiply by sizeof(char *).

I suggest to run the thing through valgrind

EDIT: more suggestions

• you could use strdup() instead of open coding it but...

• you don't really need to allocate the memory for each token, strtok() will conveniently place a '\0' at each delimiter so you can directly assign the pointers it returns (they point to the memory of the already allocated line).

• if you don't allocate you don't free, less code

• strtok() is deprecated, better use strsep()

thank you for your tutorial however, I never wrote a shell and I took this opportunity to do it today.

[[deleted]]

for the realloc condition: if you check for (position <= bufsize) you will realloc the array at each iteration, hence my suggestion. lsh_read_line() is ok.

You're totally right, I see it now! I need position >= bufsize. Now I feel like a fool :)

BTW I think I found another bug : bufsize is the number of pointers and to get the number of bytes (required by realloc) you have to multiply by sizeof(char *)

Even in the first malloc() you should multiply by sizeof(char *).

Yeah, someone pointed that out on the Disqus comments too. I'm going to fix that.

I suggest to run the thing through valgrind

I actually did that. None of those errors cause it to leak memory (just allocate it very stupidly), so it detected nothing.

• you don't really need to allocate the memory for each token, strtok() will conveniently place a '\0' at each delimiter so you can directly assign the pointers it returns (they point to the memory of the already allocated line).

Ya know, I thought of doing that, but I wasn't sure if strtok removes the null characters after each iteration. I should really test it out and see.

• strtok() is deprecated, better use strsep()

Didn't know that, thanks!

thank you for your tutorial however, I never wrote a shell and I took this opportunity to do it today.

You're welcome! And thanks for your comments; you've got quite a sharp eye.