0

I honestly have no idea how the following even happens. Here is the code:

while(1)
{
    char** line = read_command();
    char* command = line[0];
    char** parameters = malloc(100);
    int i;

    for(i = 0; i < pNum; i++) // pNum is a global var containing the number of words read from read_command()
    {
        parameters[i] = line[i];
        printf("%i: %s", i, parameters[i]);
    }

    printf("%s\n", parameters[0]);
    parameters[0] = "/usr/bin/";
    strcat(parameters[0], command);
    printf("%s\n", command);
    printf("%s\n", parameters[0]);

    if(fork() != 0)
        waitpid(1, &status, 0);
    else
        execve(parameters[0], parameters, NULL);
}

read_command() returns a char** that is basically an "array" to the string that was entered, and each char* contains a word. Like if I enter "hello people of earth" the result would be ["hello", "people", "of", "earth"]. This fucntion always works.

upon the first iteration everything works just as expected. example, when I type "date" the output is as follows:

0: date
date
date
/usr/bin/date
and then the date is displayed

but upon the second iteration if i use "date" as the input again the output is as follows:

0:date
edate 
/usr/bin/datedate 
and the date command is not issued

the second printf statement always prints "e" after the first iteration even if I print a constant string like "hello". and then the parameters[0] somehow has 2 "date" in it even though the command pointer has only 1 "date".

And after the third iteration the program does not wait for user input, it just loops non stop and displays "PM: warning, process table is full!"

What could possibly cause this?

I am working in MINIX 3.1.0 with the cc compiler for C

EDIT: the read_command():

char* line = malloc(), * linep = line;
size_t lenmax = 100, len = lenmax;
int c;
int currPos = 0;
int currParam = 0;
int i;
char** parameters = malloc(100);

if(line == NULL)
    return NULL;

while(1)
{
    c = fgetc(stdin);

    if(c == EOF) || c == '\n')
        break;

    if(--len == 0)
    {
        char* linen = realloc(linep, lenmax *= 2);
        len = lenmax;

        if(linen == NULL)
        {
            free(linep);
            return NULL;
        }

        line = linen + (line - linep);
        linep = linen;
    }

    if((*line++ = c) == '\n')
        break;
}

*line = '\0'; // everything up to this point i got from this link: http://stackoverflow.com/a/314422/509914

 parameters[currentParam] = malloc(100);

for(i = 0; i < strlen(linep); i++);
{
    if(isspace(linep[i]) || line[i] == EOF)
    {
        parameters[currParam][currPos] = '\0;
        currPos = 0;
        parameters[++currParam] = malloc(100);
    }
    else
        parameters[currParam][currPos++] = line[i];
}

parameters[currParam][currPos] = '\0';
pNum = currParam + 1;
return parameters;
Armand Maree
  • 488
  • 2
  • 6
  • 21
  • 1
    Your `printf("%i: %s", parameters[i]);` required two values, not one. It should probably be: `printf("%i: %s", i, parameters[i]);` – cdarke Aug 11 '15 at 09:20
  • 1
    `char** parameters = malloc(100);` is allocating 100 bytes, rather than 100 lots of `char *`. Perhaps you meant `char **parameters = malloc(pNum * sizeof *parameters);` – autistic Aug 11 '15 at 09:22
  • @cdarke, thanks, i changed it. It is correct in my code. – Armand Maree Aug 11 '15 at 09:23
  • 1
    In `parameters[0] = strcat("/usr/bin/", command);` you're telling `strcat` to modify the read-only string literal `"/usr/bin"`... CRASH! – autistic Aug 11 '15 at 09:23
  • @Freenode-newbostonSebivor ill change the malloc and report back now. But the strcat used to contain a pointer to a string "/usr/bin/" and then I'd assign that to parameters[0] but the same thing happend. – Armand Maree Aug 11 '15 at 09:26
  • The same thing will happen. You should be declaring those as `const char *`. – cdarke Aug 11 '15 at 09:27
  • I think we need to see `read_command`. – cdarke Aug 11 '15 at 09:31
  • 2
    "It is correct in my code" -- Why would you post anything other than your code? Surely you didn't hand type all this in rather than using copy & paste? Anyway, your code is riddled with bugs. – Jim Balter Aug 11 '15 at 09:35
  • "What could possibly cause this?" -- Your execve fails, so you loop around and create another process, and it creates another process, etc. – Jim Balter Aug 11 '15 at 09:44
  • @Freenode-newbostonSebivor "CRASH!" -- It's undefined behavior. Apparently it doesn't crash on his Minix system, but just blithely carries on, forking until it can't any more. – Jim Balter Aug 11 '15 at 09:46
  • @cdarke I added the read_command() function. and Jim Balter, I did type it all by hand, because im running a virtual machine and I dont think you can get MINIX and your host PC to share a clipboard. And I get that my execve failed, that is the problem. I've never done all this memory allocation stuff before, and the lecturer threw us in on the deep end with this project so I'm doing my best to write a decent program. – Armand Maree Aug 11 '15 at 09:51
  • You can't transfer a file to or from your Minix system? " And I get that my execve failed, that is the problem" -- You asked "how can this possibly happen", and I told you. If you don't want it to loop non-stop, then exit the loop if execve fails. If you mean "how can I possibly have a bug" ... get real. You're scribbling all over memory; anything can happen. Doesn't Minix have a debugger? – Jim Balter Aug 11 '15 at 09:56
  • They way our lecturer instructed us to set up MINIX means it has no networking capabilities. I can however shut down the VM and mount the /home folder and the transfer the file and then unmount it again then restart the VM. Believe me, if I could do it any other way I would, coz its such a hassle (or at least I dont know of any other way). I understand what you mean, I'll implement that. But the problem is that the "date" command shouldnt fail, especially not if it worked once already. – Armand Maree Aug 11 '15 at 10:01
  • The date command won't run if you filled up the process table with all those forks. And you've got all these processes doing printfs so the output will get intermixed. – Jim Balter Aug 11 '15 at 10:10
  • This is getting worse and worse. Your code blatantly fails to compile, and I'm voting to close it because the behaviour you describe isn't what we get. If you want help with this, google "stackoverflow mcve" and do some reading before you edit this, then ping me once you can show me that you know how to produce an mcve. – autistic Aug 11 '15 at 11:33
  • @Freenode-newbostonSebivor and cdarke I appreciate all the comments you guys left. I went to read up on all the stuff you guys said, especially the memory allocation. Usually when I post code it is better than this one (I think), but I was so confused I didn't understand what was going on and I was stressing coz my deadline for the project was like an hour away. I missed it (damn) but I just figured the entire thing out (it's working perfectly) and I understand it now, so the next one will go better. I will close this question because the code is horrible (compared to the code I have now). – Armand Maree Aug 11 '15 at 14:34

1 Answers1

1

It sure is interesting that those people who learn by reading reputable resources, such as the tried and tested for decades K&R tend to have these issues far less frequently than those who don't...

char** parameters = malloc(100); is attempting to allocate 100 bytes. realloc behaves similarly, so I won't mention this again. Perhaps you meant to allocate 100 lots of char *? It'd make even more sense to allocate pNum lots of char *: char **parameters = malloc(pNum * sizeof *parameters);... char* linen = realloc(linep, (lenmax *= 2) * sizeof *linep);...

strcat doesn't allocate memory; the only functions that allocate memory are malloc, calloc and realloc.

When you call strcat(foo, bar); you're asking strcat to append the string pointed to by bar onto the end of the string pointed to by foo. In your code, you're attempting to modify a string literal. Undefined behaviour, and typically a segfault.

Even your modified attempt is wrong. In parameters[0] = "/usr/bin/";, you're not copying a string into parameters[0]; you're assigning parameters[0] to point to the string (which is typically in immutable memory, as I mentioned earlier). You really need to narrow down the source of your undefined behaviour by creating an MCVE...

In the first line of your read_command() function, char* line = malloc(), * linep = line;, you have called malloc without providing an argument. That's a constraint violation. Your compiler should be issuing you with an error. Perhaps you've forgotten to #include <stdlib.h>, and so malloc is missing its prototype? Please provide an MCVE so we don't have to make guesses like this.

There's another constraint violation at if(c == EOF) || c == '\n')... Even if we were to fill in the blanks to produce an MCVE for you (which we shouldn't have to do, because it's your work and you're asking us for help), this code would not compile. Perhaps that's what's causing your crashes? Always check the messages that your compiler gives you. Don't ignore warnings... and definitely don't ignore error messages.

I compared the code that you claimed came from this answer, and it is quite different. The code at that answer compiles, for one. Nonetheless, is that your way to establish trust with the people who are trying to help you... by lying?

You don't need so much dynamic allocation, and normally I would go out of my way and explain how you can do this the best way possible, but the lies have put me off. I have one more point to make: Make sure parameters is terminated by a null pointer (similarly to how strings are terminated by a null character), as it is required to be by the manual.

Community
  • 1
  • 1
autistic
  • 1
  • 3
  • 35
  • 80
  • 1
    *the only functions that allocate memory* include `calloc` and `alloca`? – cdarke Aug 11 '15 at 09:29
  • Okay, I changed that part back to the way it was the first time I wrote it. I changed it in an attempt to find the error. check the code. The problem still persists in the EXACT same manner. :-) – Armand Maree Aug 11 '15 at 09:33
  • @ArmandMaree: You miss the point, the "first time you wrote it" was probably wrong as well. If you have a pointer to a literal, that should be a `const`. `const char * greet = "Hello";`. NEVER miss out that `const`. – cdarke Aug 11 '15 at 09:44
  • @cdarke My bad, I forgot about `calloc`... but I can't seem to find anything about *`alloca`* in [the C standard](http://www.iso-9899.info/n1570.html), so perhaps it's something that is non-C and hence, irrelevant to this question? – autistic Aug 11 '15 at 11:30