patterncMinor
Implementation of the ls command with several options
Viewed 0 times
thewithoptionsseveralimplementationcommand
Problem
As part of a past assignment, I've expanded on this implementation of
It currently supports these (described on this page):
Other than that, it doesn't list a
I've already ran this through Valgrind with and without different options, and there appears to be no memory leaks. I think it can still be structured better and simplified, though.
```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
static char* global_dir = ".";
struct Options
{
bool using_a;
bool using_d;
bool using_h;
bool using_i;
bool using_l;
bool using_p;
bool using_Q;
bool using_R;
bool using_S;
bool using_t;
bool using_U;
};
static void init_opts(struct Options* opts)
{
opts->using_a = false;
opts->using_d = false;
opts->using_h = false;
opts->using_i = false;
opts->using_l = false;
opts->using_p = false;
opts->using_Q = false;
opts->using_R = false;
opts->using_S = false;
opts->using_t = false;
opts->using_U = false;
}
struct Options get_opts(int count, char* args[])
{
struct Options opts;
init_opts(&opts);
int opt;
while ((opt = getopt(count, args, "adhilpQRStU")) != -1)
{
switch (opt)
{
case 'a': opts.using_a = true; break;
case 'd': opts.using_d = true; break;
case 'h': opts.using_h = true; break;
case 'i': opts.using_i = true; break;
case 'l': opts.using_l = true; break;
case 'p': opts.using_p = true; break;
case 'Q': opts.using_Q = true; break;
ls to have more options available, including non-standard ones.It currently supports these (described on this page):
-a
-d
-h
-i
-l
-p
-Q
-R
-S
-t
-U
Other than that, it doesn't list a
total field nor does it have coloring. It also doesn't have the same adaptive column alignments, but I have it done with fixed amounts so that it's still easier to read each column with -l.I've already ran this through Valgrind with and without different options, and there appears to be no memory leaks. I think it can still be structured better and simplified, though.
```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
static char* global_dir = ".";
struct Options
{
bool using_a;
bool using_d;
bool using_h;
bool using_i;
bool using_l;
bool using_p;
bool using_Q;
bool using_R;
bool using_S;
bool using_t;
bool using_U;
};
static void init_opts(struct Options* opts)
{
opts->using_a = false;
opts->using_d = false;
opts->using_h = false;
opts->using_i = false;
opts->using_l = false;
opts->using_p = false;
opts->using_Q = false;
opts->using_R = false;
opts->using_S = false;
opts->using_t = false;
opts->using_U = false;
}
struct Options get_opts(int count, char* args[])
{
struct Options opts;
init_opts(&opts);
int opt;
while ((opt = getopt(count, args, "adhilpQRStU")) != -1)
{
switch (opt)
{
case 'a': opts.using_a = true; break;
case 'd': opts.using_d = true; break;
case 'h': opts.using_h = true; break;
case 'i': opts.using_i = true; break;
case 'l': opts.using_l = true; break;
case 'p': opts.using_p = true; break;
case 'Q': opts.using_Q = true; break;
Solution
Looks pretty clean for the most part! Here's the source of the actual GNU implementation used on most systems for comparison (if you already haven't taken a look at it). Just a few notes:
-
Looks like part of this function could be converted into a
Would still retain the same functionality, but reduce LOC and make it more readable/concise.
-
I also noticed you have a magic number in the above function (as well as some other places). I think it best in this situation to just place a comment above its usage to specify what that number is.
-
I'm not a big fan of the
-
Prefer
-
You should use
You could use this in other places too, as I see you are starting from 0 and only counting up (meaning you'd have no negative numbers, right?). I'm not saying
-
No reason to
-
Looks like part of this function could be converted into a
for loop:void readable_fs(double size, char* buf)
{
const char* units[] = { "", "K", "M", "G", "T" };
int i = 0;
while (size > 1024)
{
size /= 1024;
++i;
}
sprintf(buf, "%.*f%s", i, size, units[i]);
}Would still retain the same functionality, but reduce LOC and make it more readable/concise.
-
I also noticed you have a magic number in the above function (as well as some other places). I think it best in this situation to just place a comment above its usage to specify what that number is.
-
I'm not a big fan of the
using_t name, since the _t is reserved for POSIX type implementations. t_option would be better? But if you change that one name you may as well be consistent and change all the others to be similar.-
Prefer
snprintf() to sprintf().-
You should use
size_t in your for loop counter variable here:for (long int i = 0; i < count; ++i)You could use this in other places too, as I see you are starting from 0 and only counting up (meaning you'd have no negative numbers, right?). I'm not saying
size_t is a good fit for every situation though, use the smallest type that will fit the situation as appropriately as possible to. stdint.h is your best friend here.-
No reason to
return 0;, can/should be removed.Code Snippets
void readable_fs(double size, char* buf)
{
const char* units[] = { "", "K", "M", "G", "T" };
int i = 0;
while (size > 1024)
{
size /= 1024;
++i;
}
sprintf(buf, "%.*f%s", i, size, units[i]);
}for (long int i = 0; i < count; ++i)Context
StackExchange Code Review Q#114479, answer score: 9
Revisions (0)
No revisions yet.