I've made vast improvements to my previous ls implementation, including:Added new flags:-g (omit owner listing)-o (omit group listing)-r (reverse with sorting)Removed a flag (which wasn't working entirely):-RAdded long options for applicable flags (as defined)Defined a single buffer sizeUsed more proper stat field typesReplaced sprintf() with snprintf()Used function pointers to remove duplication with sortingGreatly improved modularity to break up larger functionsMade opts global to work with comparer functionsActually sort in reverse order as opposed to just displaying in reverse.  This also greatly simplified the display code.I was also considering POSIX regex for the -a flag (include hidden files) and a prospective -A flag (include hidden files except for the implied . and .. directories).  Since I couldn't get it to work with the latter, I decided to leave it out.  Keeping it just for the former might've then been a little overkill.I realize that there's still no sorting done with the -d option, and I'm working on that.  If the logic around it can be simplified, then it should make it easier to change.#include <sys/stat.h>#include <sys/types.h>#include <dirent.h>#include <getopt.h>#include <grp.h>#include <pwd.h>#include <stdbool.h>#include <stdint.h>#include <stdio.h>#include <stdlib.h>#include <string.h>#include <strings.h>#include <sysexits.h>#include <time.h>#include <unistd.h>#define BUF_SIZE 1024const char* current_dir = .;struct Options{    bool using_a_opt;    bool using_d_opt;    bool using_g_opt;    bool using_h_opt;    bool using_i_opt;    bool using_l_opt;    bool using_o_opt;    bool using_p_opt;    bool using_Q_opt;    bool using_r_opt;    bool using_S_opt;    bool using_t_opt;    bool using_U_opt;} opts;void init_opts(){    opts.using_a_opt = false;    opts.using_d_opt = false;    opts.using_g_opt = false;    opts.using_h_opt = false;    opts.using_i_opt = false;    opts.using_l_opt = false;    opts.using_o_opt = false;    opts.using_p_opt = false;    opts.using_Q_opt = false;    opts.using_r_opt = false;    opts.using_S_opt = false;    opts.using_t_opt = false;    opts.using_U_opt = false;}void get_opts(int count, char* args[]){    init_opts();    static struct option long_opts[] = {        { all,             optional_argument, NULL, 'a' },        { directory,       optional_argument, NULL, 'd' },        { ,                optional_argument, NULL, 'g' },        { human-readable,  optional_argument, NULL, 'h' },        { inode,           optional_argument, NULL, 'i' },        { ,                optional_argument, NULL, 'l' },        { ,                optional_argument, NULL, 'o' },        { file-type,       optional_argument, NULL, 'p' },        { quote-name,      optional_argument, NULL, 'Q' },        { reverse,         optional_argument, NULL, 'r' },        { ,                optional_argument, NULL, 'S' },        { ,                optional_argument, NULL, 't' },        { ,                optional_argument, NULL, 'U' },        { NULL,              0,                 NULL,  0  }    };    int opt;    while ((opt = getopt_long(count, args,         adghilopQrStU, long_opts, NULL)) != -1)    {        switch (opt)        {            case 'a': opts.using_a_opt = true; break;            case 'd': opts.using_d_opt = true; break;            case 'g': opts.using_g_opt = true; break;            case 'h': opts.using_h_opt = true; break;            case 'i': opts.using_i_opt = true; break;            case 'l': opts.using_l_opt = true; break;            case 'o': opts.using_o_opt = true; break;            case 'p': opts.using_p_opt = true; break;            case 'Q': opts.using_Q_opt = true; break;            case 'r': opts.using_r_opt = true; break;            case 'S': opts.using_S_opt = true; break;            case 't': opts.using_t_opt = true; break;            case 'U': opts.using_U_opt = true; break;            case '?': exit(EX_USAGE);        }    }}void print_permissions(mode_t mode){    putchar((mode & S_IRUSR) ? 'r' : '-');    putchar((mode & S_IWUSR) ? 'w' : '-');    putchar((mode & S_IXUSR) ? 'x' : '-');    putchar((mode & S_IRGRP) ? 'r' : '-');    putchar((mode & S_IWGRP) ? 'w' : '-');    putchar((mode & S_IXGRP) ? 'x' : '-');    putchar((mode & S_IROTH) ? 'r' : '-');    putchar((mode & S_IWOTH) ? 'w' : '-');    putchar((mode & S_IXOTH) ? 'x' : '-');}void print_filetype(mode_t mode){    switch (mode & S_IFMT)    {        case S_IFREG:  putchar('-'); break;        case S_IFDIR:  putchar('d'); break;        case S_IFLNK:  putchar('l'); break;        case S_IFCHR:  putchar('c'); break;        case S_IFBLK:  putchar('b'); break;        case S_IFSOCK: putchar('s'); break;        case S_IFIFO:  putchar('f'); break;    }}void print_readable_filesize(off_t size){    const char* units[] = { , K, M, G, T };    int i;    for (i = 0; size > 1024; ++i, size /= 1024);    char filesize_buf[BUF_SIZE];    snprintf(filesize_buf, sizeof(filesize_buf),         %*jd%s , i, (intmax_t)size, units[i]);    printf( %8s, filesize_buf);}void print_time(time_t mod_time){    // get current time with year    time_t curr_time;    time(&curr_time);    struct tm* t = localtime(&curr_time);    const int curr_mon = t->tm_mon;    const int curr_yr = 1970 + t->tm_year;    // get modified time and year    t = localtime(&mod_time);    const int mod_mon = t->tm_mon;    const int mod_yr = 1970 + t->tm_year;    // determine format based on modification time    // (past six months)    const char* format = (mod_yr == curr_yr)                       && (mod_mon >= (curr_mon - 6))                           ? %b %e %H:%M                           : %b %e  %Y;    char time_buf[BUF_SIZE];    const size_t bytes_written =         strftime(time_buf, sizeof(time_buf), format, t);    if (bytes_written <= 0)    {        exit(EX_IOERR);    }    printf(%s, time_buf);}struct stat get_stats(const char* filename){    char path_buf[BUF_SIZE];    const int bytes_written =         snprintf(path_buf, sizeof(path_buf),         %s/%s, current_dir, filename);    if (bytes_written <= 0)    {        exit(EX_IOERR);    }    struct stat sb;    if (lstat(path_buf, &sb) < 0)    {           perror(path_buf);        exit(EX_IOERR);    }    return sb;}int is_dir(const char* filename){    struct stat sb = get_stats(filename);    if (lstat(filename, &sb) < 0)    {        perror(filename);        exit(EX_IOERR);    }    return sb.st_mode & S_IFDIR;}bool is_in_dir(const char* dir, const char* filename){    DIR* dfd = opendir(dir);    if (!dfd)    {        perror(dir);        exit(EX_IOERR);    }    struct dirent* dp = readdir(dfd);    while (dp)    {        if (strcmp(filename, dp->d_name) == 0)        {            closedir(dfd);            return true;        }              dp = readdir(dfd);    }    fprintf(stderr, file \'%s\' not found\n, filename);    closedir(dfd);    return false;}void print_link(const char* dir, const char* filename){    char path_buf[BUF_SIZE];    const int bytes_written =         snprintf(path_buf, sizeof(path_buf),         %s/%s, dir, filename);    if (bytes_written <= 0)    {        exit(EX_IOERR);    }    char link_buf[BUF_SIZE];    const ssize_t count =         readlink(path_buf, link_buf, sizeof(link_buf));    if (count == -1)    {        perror(readlink);        exit(EX_IOERR);    }    link_buf[count] = '\0';    if (opts.using_Q_opt)    {        printf( \%s\ -> \%s\\n, filename, link_buf);    }    else    {        printf( %s -> %s \n, filename, link_buf);    }}void print_name(const char* filename){    if (opts.using_Q_opt)    {        printf( \%s\, filename);    }    else    {        printf( %s, filename);    }    if (opts.using_p_opt && is_dir(filename))    {        putchar('/');    }    putchar('\n');}void display_stats(const char* dir, const char* filename){    if (!is_in_dir(dir, filename))    {        return;    }    if (!opts.using_g_opt         && !opts.using_o_opt         && !opts.using_l_opt)    {        if (opts.using_Q_opt)        {            printf(\%s\\n, filename);        }        else        {            printf(%s\n, filename);        }        return;    }    struct stat sb = get_stats(filename);    // print inode number    if (opts.using_i_opt)    {        printf(%ju , (uintmax_t)sb.st_ino);    }    print_filetype(sb.st_mode);    print_permissions(sb.st_mode);    printf( %jd , (intmax_t)sb.st_nlink);    // print owner    if (!opts.using_g_opt)    {        printf(%10s , getpwuid(sb.st_uid)->pw_name);    }    // print group    if (!opts.using_o_opt)    {        printf(%10s, getgrgid(sb.st_gid)->gr_name);    }    // print file size    if (opts.using_h_opt)    {        print_readable_filesize(sb.st_size);    }    else    {        printf(%10jd , (intmax_t)sb.st_size);    }    print_time(sb.st_mtime);    // print name/link    if ((sb.st_mode & S_IFMT) == S_IFLNK)    {        print_link(dir, filename);    }    else    {        print_name(filename);    }}int compare_size(const void* p1, const void* p2){    const char* str1 = *(const char**)p1;    const char* str2 = *(const char**)p2;    const off_t size1 = get_stats(str1).st_size;    const off_t size2 = get_stats(str2).st_size;    const int return_value = (size2 > size1) - (size2 < size1);    return (!opts.using_r_opt) ? return_value : -return_value;}int compare_time(const void* p1, const void* p2){    const char* str1 = *(const char**)p1;    const char* str2 = *(const char**)p2;    const time_t time1 = get_stats(str1).st_mtime;    const time_t time2 = get_stats(str2).st_mtime;    const int return_value = (time2 > time1) - (time2 < time1);    return (!opts.using_r_opt) ? return_value : -return_value;}int compare_lex(const void* p1, const void* p2){    const char* str1 = *(const char**)p1;    const char* str2 = *(const char**)p2;    return (!opts.using_r_opt)         ? strcasecmp(str1, str2)        : -strcasecmp(str1, str2);}void sort_dir(char** entries, size_t entries_count){    int(*comparer)(const void*, const void*);    if (opts.using_S_opt)    {        comparer = compare_size;    }    else if (opts.using_t_opt)    {        comparer = compare_time;    }    else    {        comparer = compare_lex;    }    qsort(entries, entries_count, sizeof(char*), comparer);}void structure_dir_entries(DIR* dfd, char** entries, size_t* entries_count){    struct dirent* dp = readdir(dfd);    *entries_count = 0;    while (dp)    {        const bool omit_hidden = !opts.using_a_opt                              && dp->d_name[0] == '.';        if (!omit_hidden)        {            if (*entries_count >= BUF_SIZE)            {                entries = realloc(                    entries, sizeof(entries) * BUF_SIZE);                if (!entries)                {                    perror(realloc);                    abort();                }            }            entries[*entries_count] = dp->d_name;            ++(*entries_count);        }        dp = readdir(dfd);    }    if (!opts.using_U_opt)    {        sort_dir(entries, *entries_count);    }}void display_dir_entries(const char* dir){    char** entries = malloc(BUF_SIZE * sizeof(char*));    if (!entries)    {        perror(malloc);        abort();    }    DIR* dfd = opendir(dir);    size_t entries_count;    structure_dir_entries(dfd, entries, &entries_count);    for (size_t i = 0; i < entries_count; ++i)    {        display_stats(dir, entries[i]);    }    closedir(dfd);    free(entries);}void display_dir_args(int argc, char* argv[]){    const bool no_dirs_given = (argc - optind) == 0;    if (no_dirs_given)    {        display_stats(., .);    }    for (int i = optind; i < argc; ++i)    {        display_stats(., argv[i]);    }}void scan_dir_entries(int argc, char* argv[]){    const bool multiple_dirs = (argc - optind) >= 2;    for (int i = optind; i < argc; ++i)    {        if (!is_dir(argv[i]))        {            display_stats(., argv[i]);            continue;        }        if (!is_in_dir(., argv[i]))        {            continue;        }        if (multiple_dirs)        {            printf(\n%s:\n, argv[i]);        }        display_dir_entries(argv[i]);    }}int main(int argc, char* argv[]){    get_opts(argc, argv);    if (opts.using_d_opt)    {        display_dir_args(argc, argv);        return EX_OK;    }    else    {        if (optind == argc)        {            display_dir_entries(.);            return EX_OK;        }        scan_dir_entries(argc, argv);    }}
Implementation of the ls command with several options - follow-up
c;file system;linux;posix;c99
This looks pretty good over all, but I have a few things that jumped out at me.Bugsstructure_dir_entries has a leak and likely a crash in it if BUF_SIZE actually overflows. You're basically doing this:void f(char* buf) {    buf = malloc(1);}void use_f(void) {    char* b = NULL;    f(b);    free(b);}It looks like you need to check for absolute paths:    corbin@mint ~/Desktop $ ./ls ..    Desktop    dev    Documents    Downloads    corbin@mint ~/Desktop $ ./ls /home/corbin    .//home/corbin: No such file or directorySemantics / Technical ImprovementsYour options should have names. using_a_opt is meaningless, when it could instead be named what the option does. Not only will this make the code easier to understand, it will tuck away the logic of flag <-> option mapping in the option parsing code.Imagine you didn't write the code, and you came across this:if (!opts.using_g_opt     && !opts.using_o_opt     && !opts.using_l_opt){You'd almost certainly be confused and likely curse the previous programmer a bit.opts shouldn't be a global. I know it's slightly convenient to have it be a global, but it creates a strange dependency where you have to remember to initialize it before using it. It seems inappropriate for the code consuming something to initialize it. It'd make more sense for it to be non-global, then the initialization and creation would be closer. Also, if you were to add unit testing to this, the global is likely going to complicate that.A lot of your functions handle program flow and shouldn't. For example, your functions that handle getting file metadata shouldn't kill program execution. This makes unit testing annoying, and it makes the functions less usable.It almost certainly doesn't matter since this is a single file executable and not a library, but any function you don't want to purposely expose should be marked static (i.e. probably all of your functions except for main). It's much better to over-static than leave something non-static and find out someone is using it when you try to actually encapsulate it.There's some mixture of logic and output that shouldn't be there. For example, is_in_dir returns a boolean to signify if the file is in the directory or not. I wouldn't expect this function to output anything. The caller should output something if the caller wants output. Functions shouldn't handle both logic and output. They should handle one or the other (except of course for output functions having logic about output :p).It's weird that print_link can fail. It might make more sense if you had some layer of code that would make and populate a struct, then print_link could operate on that in a way that it cannot fail. If I inherited some code and a print_* function had a chance of exiting the program, I would be pretty surprised. In a way, this is just a special case of functions should do either printing or logic, not both (and that's just a special case of a function should only do one thing).This also applies to a few other places too. For example, display_stats could take all of the data it needs instead of gathering it. The compare_* functions also seem a bit odd.As a bonus of operating on the actual data and not what are essentially ways to obtain the data, you only have to obtain it once. In other words, you can get rid of a lot of the get_stats calls, which are likely relatively expensive (though there is of course a relatively small memory premium you pay for this). This also has a nice effect of getting rid of some race conditions. For example, currently, if you call get_stats twice, the file might not exist the second time. If you only collect data up front once, you might print data that is out of date, but it will never be inconsistent.UsabilityThere's a lot of exit scenarios that don't provide a very useful error to the user. I'd hate to have to go look up what the exit codes mean every time a program died on me. It'd be a bit friendlier to provide an error message along with exit code.A --help option would be nice.Styleint opt;while ((opt = getopt_long(count, args,     adghilopQrStU, long_opts, NULL)) != -1){Whenever you have a loop like this, you can use a for loop to keep the loop variable scoped more tightly:for (int opt; (opt = getopt_long(...)) != -1;)Another example where you could use a for loop like this is in is_in_dir.It looks like you're using an 80 character line limit. This obviously comes down to personal preference (even more than other style suggestions), but I think you should consider a larger limit. There's a lot of expressions squished down into 2-4 lines, that could be 1 or 2 lines instead. I know some people like to have 2 or even 3 things up and tiled horizontally while they work, but I think even with that, the limit could be 120.There's now a 90% chance someone will yell at me.