HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

List directories (with information)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withlistinformationdirectories

Problem

For part of a project, I was given this directive:


The “ls” command will require you to use directory functions to list
out what the filenames are in the directory. Functions required to
make a “ls” command work are included in the ` header file (
man dirent ). You will need to open a directory and iterate through
the directory until you hit the end and display the filenames in that
current directory.
Now, the requirement for this piece of the lab requires an example of
“ls –l” where you display most of the inode information on the file.

I've implemented this below. It seems to mimic
ls -la quite well, even for all the edge cases I've been testing.

ls.c:

``
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include

#ifndef streq
#define streq(x, y) (strcmp((x), (y)) == 0)
#endif

/*
* This function takes a mode value and a char array
* and puts into the char array the file type and the
* nine letters that correspond to the bits in mode.
* NOTE: It does not code setuid, setgid, and sticky
* codes
*/
char* mode_to_letters(int mode)
{
const int STR_SIZE = 11; // size of mode string + terminator
char *str = malloc(STR_SIZE);
memcpy(str, "----------", STR_SIZE); // default

if (S_ISDIR(mode)) str[0] = 'd'; // directory
if (S_ISCHR(mode)) str[0] = 'c'; // char devices
if (S_ISBLK(mode)) str[0] = 'b'; // block device

if (mode & S_IRUSR) str[1] = 'r'; // 3 bits for user
if (mode & S_IWUSR) str[2] = 'w';
if (mode & S_IXUSR) str[3] = 'x';

if ( mode & S_IRGRP ) str[4] = 'r'; // 3 bits for group
if ( mode & S_IWGRP ) str[5] = 'w';
if ( mode & S_IXGRP ) str[6] = 'x';

if ( mode & S_IROTH ) str[7] = 'r'; // 3 bits for other
if ( mode & S_IWOTH ) str[8] = 'w';
if ( mode & S_IXOTH ) str[9] = 'x';

return str;
}

// returns pointer to username associated with uid, uses getpw()
char* uid_to_name( uid_t uid )
{

Solution

Don't used dynamic memory when the stack will do just fine

uid_to_name() and gid_to_name() may end up mallocing the returned string. This only complicates things (caller has to free) and decreases performance. Your code could leak memory because of that:

fprintf(stdout, "%s %2d %-9s %-6s %5lld %.12s %s\n",
        modestr,
        info_p->st_nlink,
        uid_to_name(info_p->st_uid),
        gid_to_name(info_p->st_gid),
        info_p->st_size,
        4 + ctime(&info_p->st_mtime),
        filename);


You've only freed modestr.

So why not pass in the buffer instead?

char* uid_to_name(uid_t uid, char* buffer, size_t bufSize);
char* gid_to_name(gid_t gid, char* buffer, size_t bufSize);


Then those could be called with a stack buffer:

char uid_name_buf[10];
char gid_name_buf[10];

fprintf(stdout, "%s %2d %-9s %-6s %5lld %.12s %s\n",
        modestr,
        info_p->st_nlink,
        uid_to_name(info_p->st_uid, uid_name_buf, sizeof(uid_name_buf)),
        gid_to_name(info_p->st_gid, gid_name_buf, sizeof(gid_name_buf)),
        info_p->st_size,
        4 + ctime(&info_p->st_mtime),
        filename);


A little more verbose, but not really, since for the current code to be fully correct you'd have to free the returned pointer if it was allocated. The same thing can be applied to mode_to_letters().

Miscellaneous:

-
Mark pointers you are only reading from as const, specially function parameters! This is very important to convey the correct intent!

-
Prefer snprintf since it will check for overflow - you're not using it consistently.

-
Every function but main could be static, but I'm just being pedantic.

-
streq macro was never used, so could be removed.

-
I see some minor inconsistencies with how () are spaced - some have extra whitespace padding and others don't - ClangFormat to the rescue?.

-
int main(int ac, char *av[]) - you've misspelled argc and argv ;)

Code Snippets

fprintf(stdout, "%s %2d %-9s %-6s %5lld %.12s %s\n",
        modestr,
        info_p->st_nlink,
        uid_to_name(info_p->st_uid),
        gid_to_name(info_p->st_gid),
        info_p->st_size,
        4 + ctime(&info_p->st_mtime),
        filename);
char* uid_to_name(uid_t uid, char* buffer, size_t bufSize);
char* gid_to_name(gid_t gid, char* buffer, size_t bufSize);
char uid_name_buf[10];
char gid_name_buf[10];

fprintf(stdout, "%s %2d %-9s %-6s %5lld %.12s %s\n",
        modestr,
        info_p->st_nlink,
        uid_to_name(info_p->st_uid, uid_name_buf, sizeof(uid_name_buf)),
        gid_to_name(info_p->st_gid, gid_name_buf, sizeof(gid_name_buf)),
        info_p->st_size,
        4 + ctime(&info_p->st_mtime),
        filename);

Context

StackExchange Code Review Q#149516, answer score: 7

Revisions (0)

No revisions yet.