-1

In my 1st year of programming, I've been asked to write a program to mimic a shell in c, with some basic commands.

At some point, I've tried to implement the "cd" command to switch directories, and I had a question about the size I had to give to the "getcwd" function.

In the man page, it's written that "getcwd" has to be given a path and a size to know where to store the path and on how many bytes.

I've tried to get the size depending on the length of the path and the bytes of a character and it worked quite well in the beginning, but at some point, I had some issues with an existing bigger path than my current path I was trying to go to.

For example, I was in /home/$username/..... and I tried to go to "/another_directory" inside of "/home/$username/", but without typing the path from the beginning, causing it not to know the full length of the directory but only my input, and not working.

A friend of mine helped me out telling me I could use "size_t" (as shown in the "man" page too)

I'd like to know how "size_t" works in this case because it's just allocating a big enough size making everything working fine without even malloc-ing it myself.

Here's the code :

{
    size_t len;
    char *path = NULL;

    if (buffarg == 1) {
        chdir("/home/cyprien"); //TODO modify cyprien to $NAME
        my_putstr("/home/cyprien\n");
    } else if (buffarg == 2) {
        path = malloc(len);
        if (chdir(buffarray[1]) == 0) {
            getcwd(path, len);
            my_putstr(path);
            my_putstr("\n");
        } else {
            my_putstr("cd: no such file or directory: ");
            my_putstr(buffarray[1]);
            my_putstr("\n");
        }
    }
    free(path);
}

I initialized size_t len; and used it in getcwd(path, len) and it always finds the right size. My older code was something like this :

{
    int len = 0;
    char *path = NULL;

    if (buffarg == 1) {
        chdir("/home/cyprien"); //TODO modify cyprien to $NAME
        my_putstr("/home/cyprien\n");
    } else if (buffarg == 2) {
        path = malloc(len);
        if (chdir(buffarray[1]) == 0) {
            len += my_strlen(buffarray[1]);
            getcwd(path, len);
            my_putstr(path);
            my_putstr("\n");
        } else {
            my_putstr("cd: no such file or directory: ");
            my_putstr(buffarray[1]);
            my_putstr("\n");
        }
    }
    free(path);
}

I was initializing len as an int to 0, and trying to get the length of my input to add to size in order to store my buffer in the path string.

Does anyone knows how size_t works in this case ? I was really thinking about this since yesterday and I'd like to know more about that.

Sorry for such long text for a such small "problem"

pironc
  • 51
  • 1
  • 9
  • 1
    The first version is definitely not better as it is very wrong. In that version `len` is never set so it contains a random garbage value. Which may happen to be a value that results in passing a usable value to `malloc`. But that is in no way correct as it is undefined behaviour (ie luck). The second version is also wrong because it allocates 0 memory. Your issue has nothing to do with `size_t`. You just need to call `malloc` **after** the `my_strlen` call so that the right buffer size can be allocated. Be sure to add 1 to account for the terminating NUL. – kaylum Apr 14 '20 at 10:31
  • Though I don't know what `my_strlen(buffarray[1])` is actually doing so I'm not sure even that is correct. If you need further help you need to provide a [minimal verifiable example](https://stackoverflow.com/help/minimal-reproducible-example) and give the exact input, expected result and actual result. – kaylum Apr 14 '20 at 10:33
  • Both of them are nonsense. In the first you use `len` without initializing it. In the second, you use `malloc` to allocate zero bytes. What's the point with that? – klutt Apr 14 '20 at 10:34
  • Well, I'm not 100% sure if this was my exact code as I modified its' goal was to find the number of bytes in my second given parameter (here it is cd then a path) so I could find 5 bytes for "/home" for example (without the quotes) and add more bytes to know how much to write in path. "my_strlen" just reads until the end and adds one each time. – pironc Apr 14 '20 at 10:35
  • You're not making things very clear. I would guess there are concepts you are not understanding and hence not able to use the right words. That's why it is critical on Stack Overflow to provide the requested info: MVE, input, expected result, actual result. That is the clearest way to describe your problem without ambiguity (proven over 10s of thousands of SO posts). – kaylum Apr 14 '20 at 10:38
  • It works fine without initializing len, but I guess it's much better to give it a value instead of nothing like I did. Do you initialize it like an int, to zero for example? And also, I tried to modify the code for both of them and not mallocing len with ```path = malloc(len)``` makes my program segfault. – pironc Apr 14 '20 at 10:41
  • NO! It doesn't work fine. That's what we are trying to explain to you. Using inintialised variables is called "undefined behaviour" in C. With UB it can sometimes appear to "work" but when run again, or with a different compile or on a different system etc, it may exhibit different symptoms. That is, it's pure chance. Think about it - does it seem like it even makes sense to use a variable with an unknown value? – kaylum Apr 14 '20 at 10:47
  • 1
    I guess this would help you understand a bit better what my code does : https://imgur.com/DdYs07C.png Basically, I just want to use the ```getcwd``` command to find my path and store it somewhere to write it in my shell. I was struggling to know what size to give it, how to malloc it, if I even had to, etc.. I'm not very experienced and doesn't know much about allocating, sizes, size_t struct and so on. I tried to find ways to write my program and so far this is what I did. I also just joined SO and I'm not sure on how to show people what I got, what's not really working, etc.. – pironc Apr 14 '20 at 10:47
  • https://stackoverflow.com/q/9449241/905902 `PATH_MAX` exists for a reason – wildplasser Apr 14 '20 at 10:50
  • @pironc *"Do you initialize it like an int, to zero for example?"* - Yes, I ALWAYS initialize ALL variables before I read from them. Doing otherwise is undefined behavior. – klutt Apr 14 '20 at 11:30
  • @pironc When you invoke `path = malloc(len);` How many bytes do you expect to allocate if `len` is A) uninitialized and B) set to zero? – klutt Apr 14 '20 at 11:43
  • I also always initialize my variables in all cases, but in this case it was working fine without initializing it (which is not good, of course because it could crash in other shells, compilers, etc..) If I set my size_t len uninitialized I "expect it" to malloc a set number of bytes according to what has to be read. At first I was like "well it's not going to do anything as it's zero or uninitialized" but it worked. And if it's set to zero, it's as initializing it as an int to zero, and doing my usual mallocs as before. I talked with friends and I saw what was the problem (ill put it after) – pironc Apr 14 '20 at 12:33
  • @pironc If you do `malloc(0)` you will allocate zero bytes, which is a completely pointless operation to do. And how should an uninitialized variable make `malloc` able to guess how much memory it needs to allocate to fit the input of a future operation? I'm sorry, but it makes no sense at all. – klutt Apr 14 '20 at 12:47
  • I probably just left it from the newer version I wrote with the uninitialized size_t, as I didn't have access to my older version, but it makes sense that it's useless for the second program. I found my problem and published the answer down below, I could just use NULL for the path and 0 for the size to automatically allocate them depending on what I was writing / the actualy full path I was in. It's much simpler and without issues now. – pironc Apr 14 '20 at 17:06

1 Answers1

0

I discovered know what's wrong in my code.

I was trying to read my current path directory and store it in an array with a size that I was trying to read according to my paths' length.

BUT, I didn't correctly understand the way getcwd worked :

If the buffer of getcwd is set to NULL and the size of getcwd is set to 0, the buffer will be malloc'd of an array with the right size of the input given for the current working directory.

In other words, nothing has to be done manually, but just by giving my NULL initialized array to getcwd to return to, NULL as the buffer and 0 in size, like so :

Initialized a buffer at the beginning : char *path = NULL;

and return getcwd in path : path = getcwd(NULL, 0);

My code is now like this, working fine and all the variables initialized properly :

void cd(char **buffarray, int buffarg)
{
    char *path = NULL;

    if (buffarg == 1) {
        chdir("/home/cyprien"); //TODO modify cyprien to $NAME
        my_putstr("/home/cyprien\n");
    } else if (buffarg == 2) {
        if (chdir(buffarray[1]) == 0) {
            path = getcwd(NULL, 0);
            my_putstr(path);
            my_putstr("\n");
        } else {
            my_putstr("cd: no such file or directory: ");
            my_putstr(buffarray[1]);
            my_putstr("\n");
        }
    }
    free(path);
}
pironc
  • 51
  • 1
  • 9