I try to program a simple implementation of ls for school

#include <dirent.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
int normalsort( const void *string1, const void *string2) {
    char *const *char1 = string1;
    char *const *char2 = string2;
    return strcasecmp(*char1, *char2);
}

int listDir (char* path, int listToggeled, int classifyToggeled){
    DIR *dir = opendir(path);
    struct dirent *entry;
    struct stat s;
    char** listofentries;
    char symbol = '-';
    int counter = 0;





    while ((entry = readdir(dir)) != NULL){
        if (entry->d_name[0] != '.'){
            listofentries = realloc(listofentries, (counter   1) * sizeof(char*));
            listofentries[counter] = entry->d_name;
            counter  ;
        }
    }

    qsort(listofentries, counter, sizeof(char*), normalsort);

    for (int i = 0; i < counter; i  ){
        char* entryname = listofentries[i];
        if (entryname[0] != '.'){
            printf("%s", entryname);

            if (classifyToggeled == 1){
                lstat(entryname, &s);

                if (S_ISDIR(s.st_mode)) {
                    symbol = '/';
                } else if (S_ISLNK(s.st_mode)) {
                    symbol = '@';
                } else if ((S_ISREG(s.st_mode)) && (s.st_mode & S_IXUSR)) {
                    symbol = '*';
                } else {
                    symbol = ' ';
                }

                printf("%c", symbol);
            }

            if (listToggeled == 1){
                printf("\n");
            }
            else {
                printf("  ");
            }
        }
    }
    closedir(dir);
    return 0;
}

int main(int argc, char **argv) {
    int classifyToggeled = 0;
    int listToggeled = 0;
    char* dirToList = ".";

    if (argc == 1){
        listDir(dirToList, listToggeled, classifyToggeled);
        return 0;
    }
    for (int i = 1; i < argc; i  ){
        char* currentArg = argv[i];

        //Check if -F is set
        if (strcmp(currentArg, "-F") == 0 || strcmp(currentArg, "-1F") == 0 || strcmp(currentArg, "-F1") == 0){
            classifyToggeled = 1;
        }

        //Check if -1 is set
        if (strcmp(currentArg, "-1") == 0 || strcmp(currentArg, "-1F") == 0 || strcmp(currentArg, "-F1") == 0){
            listToggeled = 1;
        }

        //List out all folders
        if (currentArg[0] != '-'){
            dirToList = currentArg;
        }

    }
    //If no folders were listed yet, list current folder
    //printf("dirtolist: %s", dirToList); <-- This line
    listDir(dirToList, listToggeled, classifyToggeled);
    if (listToggeled == 0){
        printf("\n");
    }
    return 0;
}

It has a few bugs:

• When the printf line marked above is not commented out, the program tells realloc(): invalid old size
• When this line is commented out this only happens if the program is executed without any parameters
• /bin/ and /sbin/ print out weird characters

I think this is some kind of memmory issue but I have hardly any C knowledge to just see it

CodePudding user response：

Your code exhibits undefined behaviour because you passed a non-static, uninitialised pointer to realloc whose contents were indeterminate.

From C11, Memory Management Functions:

The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size. ....if ptr does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to the free or realloc function, the behavior is undefined.

Possible fixes:

• Initialize it with 0 / NULL.
• Declare it as static.¹
• Replace it with a call to malloc.

The realloc function returns a pointer to the new object (which may have the same value as a pointer to the old object), or a null pointer if the new object could not be allocated.

realloc returns NULL on failure, your code should check for it.

#include <errno.h>

char *line = 0;
errno = 0;

char *new = realloc (line, size);
if (!new) {
    perror ("realloc");
    /* realloc() failed to allocate memory, handle error accordingly. */
}

The above code snippet can be rewritten as:

#include <errno.h>

errno = 0;

char *new = malloc (size);
if (!new) {
    perror ("malloc");
    /* malloc() failed to allocate memory, handle error accordingly. */
}

Similarly, opendir returns a NULL pointer while lstat returns -1 to indicate an error. Check for them.

[1] — Objects declared with static storage duration and are always initialised. In the absence of a definition, they are implicitly initialised to 0.

From the C standard C11 6.7.9/10:

"... If an object that has static or thread storage duration is not initialized explicitly, then:

— if it has pointer type, it is initialized to a null pointer;

— if it has arithmetic type, it is initialized to (positive or unsigned) zero;"

Though, it's good practice to explicitly set it to 0.

CodePudding user response：

Bugs:

• listofentries isn't initialized, it must be set to NULL or you can't use realloc.
• You don't check if realloc succeeded or not.