Cleaning up elkscmd and adding help text - Questions

Nils Stec
2015-03-27T08:19:56Z

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

Hi,

i started to write some patches, at the moment 2. Now it's the time to
ask you guys some questions about that.

I wrote a patch for cat which shows the user some usage-information.
Jody Bruchon used write() for this purpose, other tools are using
fprintf(stderr, ....).
I don't know which i should prefer. In my opinion i would rather use
fprintf.
STDERR should be the output for operations, right?

I wrote a second patch, which adds some more functionality to cp.
It adds three command-line-options:
-v be verbose
-i be interactive, ask before overwriting
-n don't overwrite files

i want to add 3 more options:
-u update-only (only overwrite if source is newer)
-b make a backup if destination file already exists
-f force, if destfile cannot be opened, remove it, try again.

Is this the right way to write code for this project? It's the first
time I'm doing this in here and i don't wan't to do things wrong...

I think these two patches are still just drafts - i send them because i
want to show, i know theres more work to do on this two patches.

Nils

[-- Attachment #2: patch-usage-cat.diff --]
[-- Type: text/x-patch, Size: 829 bytes --]

diff --git a/elkscmd/file_utils/cat.c b/elkscmd/file_utils/cat.c
index e8b9576..d4616c7 100644
--- a/elkscmd/file_utils/cat.c
+++ b/elkscmd/file_utils/cat.c
@@ -35,6 +35,8 @@ int main(int argc, char **argv)
if (dumpfile(STDIN_FILENO)) goto error_read;
} else {
for (i = 1; i < argc; i++) {
+ if(!strcmp(argv[1], "-h")) goto usage;
+
errno = 0;
fd = open(argv[i], O_RDONLY);
if (fd == -1) {
@@ -52,4 +54,11 @@ error_read:
argv[0], argv[i], strerror(errno));
close(fd);
exit(1);
+
+usage:
+ write(STDERR_FILENO, "concatentate file(s) or standard input to standard output.\n", 60);
+ write(STDERR_FILENO, "usage: cat [OPTIONS] [FILE]\n", 29);
+ write(STDERR_FILENO, "options: -h help\n", 18);
+ write(STDERR_FILENO, "when no file is given or file is '-' STDIN is used for input.\n", 62);
+ exit(0);
}

[-- Attachment #3: patch-cp-add_options_001.diff --]
[-- Type: text/x-patch, Size: 4523 bytes --]

diff --git a/elkscmd/file_utils/cp.c b/elkscmd/file_utils/cp.c
index 6e23864..ed1e66c 100644
--- a/elkscmd/file_utils/cp.c
+++ b/elkscmd/file_utils/cp.c
@@ -22,9 +22,16 @@
#include <errno.h>

#define BUF_SIZE 4096
+#define NOT_OVERWRITING 0xABCD

static char *buf;

+static unsigned int copy_count;
+
+/* we need some flags for command-line-options */
+static int option_no_overwrite;
+static int option_verbose;
+static int option_interactive;

/*
* Build a path name from the specified directory name and file name.
@@ -79,14 +86,26 @@ int copyfile(char *srcname, char *destname, int setmodes)
struct stat statbuf2;
struct utimbuf times;

- if (stat(srcname, &statbuf1) < 0) {
+ if (stat(srcname, &statbuf1) < 0) { // src nonexistent
perror(srcname);
return 0;
}

- if (stat(destname, &statbuf2) < 0) {
+ if (stat(destname, &statbuf2) < 0) { // dst nonexistent
statbuf2.st_ino = -1;
statbuf2.st_dev = -1;
+ } else { // dst existent
+ if(option_no_overwrite) {
+ fprintf(stderr, "dest '%s' is existent, not overwriting\n", destname);
+ return NOT_OVERWRITING;
+ }
+ if(option_interactive) {
+ /* TODO fix this, getchar wants return and seems not to work properly */
+ //printf("really overwrite '%s'? (y/n) \n", destname);
+ //if(getchar() != 'y') {
+ // return NOT_OVERWRITING;
+ //}
+ }
}

if ((statbuf1.st_dev == statbuf2.st_dev) &&
@@ -143,6 +162,12 @@ int copyfile(char *srcname, char *destname, int setmodes)
(void) utime(destname, &times);
}

+ if(option_verbose) {
+ printf("copied '%s' -> '%s'\n", srcname, destname);
+ }
+
+ copy_count++;
+
return 1;

@@ -157,37 +182,78 @@ error_exit:
int main(int argc, char **argv)
{
int dirflag;
+ int arg_counter;
+ int options_counter;
+ int retval;
+ unsigned int file_count;
char *srcname;
char *destname;
char *lastarg;

if (argc < 3) goto usage;

- lastarg = argv[argc - 1];
+ lastarg = argv[argc - 1]; // lastarg is copy destination

- dirflag = isadir(lastarg);
+ dirflag = isadir(lastarg); // is destination a directory?
+
+ // set defaults for options //
+ option_no_overwrite = 0;
+ option_verbose = 0;
+ option_interactive = 0;
+
+ // walk throug all args, see if there are options for copy-operation
+ // count options for later addition to arg-counter
+ arg_counter = 1; // start at argv[1]
+ options_counter = 0;
+ while(arg_counter < argc) {
+ if(!strcmp(argv[arg_counter], "-n")) {
+ options_counter++;
+ option_no_overwrite = 1;
+ }
+ if(!strcmp(argv[arg_counter], "-v")) {
+ options_counter++;
+ option_verbose = 1;
+ }
+ if(!strcmp(argv[arg_counter], "-i")) {
+ options_counter++;
+ option_interactive = 1;
+ }
+ if(!strcmp(argv[arg_counter], "-h")) goto usage;
+ arg_counter++;
+ }

- if ((argc > 3) && !dirflag) {
- fprintf(stderr, "%s: not a directory\n", lastarg);
+ if((argc > (3+options_counter)) && !dirflag) { // can't copy more than one src-files into one dst-file
+ fprintf(stderr, "%s is not a directory\n", lastarg);
exit(1);
}

buf = malloc(BUF_SIZE);
- while (argc-- > 2) {
- srcname = argv[1];
+ copy_count = 0;
+ file_count = 0;
+ while (argc-- > (2+options_counter)) {
+ srcname = argv[options_counter+1+file_count];
destname = lastarg;
- if (dirflag) destname = buildname(destname, srcname);
+ if(dirflag) destname = buildname(destname, srcname);

- if (!copyfile(*++argv, destname, 0)) goto error_copy;
+ retval = copyfile(srcname, destname, 0);
+ if((!retval) && (!(retval == NOT_OVERWRITING))) goto error_copy;
+ file_count++;
}
free(buf);
+ if(option_verbose) printf("copy-count = %d\n", copy_count);
exit(0);

error_copy:
fprintf(stderr, "Failed to copy %s -> %s\n", srcname, destname);
exit(1);
usage:
- fprintf(stderr, "usage: %s source_file dest_file\n", argv[0]);
- fprintf(stderr, " %s file1 file2 ... dest_directory\n", argv[0]);
+ fprintf(stderr, "usage: %s [options] source_file dest_file\n", argv[0]);
+ fprintf(stderr, " %s [options] file1 file2 ... dest_directory\n", argv[0]);
+ fprintf(stderr, "options: -b make a backup of each exisiting destination file\n");
+ fprintf(stderr, " -f if a existing destination file cannot be opened, remove it and try again\n");
+ fprintf(stderr, " -n no overwriting\n");
+ fprintf(stderr, " -v be verbose\n");
+ fprintf(stderr, " -i interactive, prompt before overwrite\n");
+ fprintf(stderr, " -u copy only if source file is newer than destination file\n");
exit(1);
}