Request critique of first program

Hi,

I'm new to C and would appreciate any feedback on the following
program, asplit, which splits a file into 2 new files, putting a
certain number of lines in the first file, and all the rest in the
second file.

Any comments as to non-portability, stylistic infelicities, outright
bugs or anything else would be very much appreciated.

It compiles fine using gcc 4.1.2, and --std=c99 -pedantic -Wall, with
no warnings, and seems to work.

There are two files, a header and a source file.

## Begin asplit.h ##

#ifndef ASPLIT_H_
#define ASPLIT_H_

enum e_status { SUCCESS, ERROR_FILE_INPUT, ERROR_FILE_OUTPUT_A,
ERROR_FILE_OUTPUT_B };

typedef enum e_status status;

typedef struct {
int status;
long int num_rem_lines;
} asplit_result;

/**
* Asymmetrically splits file named in_file into 2 files, out_file_a,
and
* out_file_b, putting num_lines (must be non-negative) lines in the
first
* file, and the rest in the second file.
*
* Any of the filenames may be "-", indicating stdin in the case of
in_file,
* or stdout in the case of out_file_a or out_file_b.
*
* Returns a pointer to an asplit_result struct (unless unable to
allocate,
* in which case it returns NULL), which shows success or failure and
the
* number of lines that were included in 2nd file if successful.
* Possible status values are given by the e_status enumeration in
this
* header file. Success is indicated by SUCCESS, and each of the 3
different
* failure states is indicated by one of the enum values beginning
with
* ERROR_, indicating which file was unable to be opened.
*
* If the status is SUCCESS, then the num_rem_lines value indicates
* the number of lines remaining after the first file was filled,
* all of which were entered into the second file.
*
* The client is responsible for memory management of the result
struct.
*
*/
asplit_result * asplit(const char *in_file, const char *out_file_a,
const char *out_file_b,
const long int num_lines);

#endif /*ASPLIT_H_*/

## End asplit.h ##

## Begin asplit.c ##

#include <stdio.h>
#include <stdlib.h>

#include <string.h>

#include <getopt.h>

#include "asplit.h"

char *copy_string(const char *const s);

int close_file(FILE * f);

FILE * open_infile(const char * fname);

FILE * open_outfile(const char *fname);

/* Name under which program was invoked. */
char *program_name;

void usage(int status) {

if (status != EXIT_SUCCESS)
fprintf(stderr, "Try `%s -h' for more information.\n",
program_name);
else {
printf(
"\n\
Usage: %s [OPTION] -i IN_FILE -a OUT_FILE_A -b OUT_FILE_B -n NUM_LINES
\n\n\
",
program_name);
puts("\
Asymmetrically split IN_FILE into 2 files, putting the first NUM_LINES
\n\
into OUT_FILE_A and the rest into OUT_FILE_B.\n\
\n\
-i input file, IN_FILE\n\
-a name of first output file, OUT_FILE_A\n\
-b name of second output file, OUT_FILE_B\n\
-n the number of lines to put in OUT_FILE_A\n\
\n\
-h display this help and exit\n\
-v verbose output\n\
");
puts("\
\n\
When IN_FILE is -, use standard input; when OUT_FILE_A or OUTFILE_B is
-,\n\
use standard output.\n\
");
printf(
"\
\n\
Examples:\n\n\
%s -n 100 -i - -a output1 -b output2\n\
Read from standard input, writing first 100 lines to output1
file and\n\
all following lines to output2.\n\n\
%s -n 1000 -i infile.txt -a output1.txt -b -\n\
Read from infile.txt, writing the first 1000 lines to
output1.txt and\n\
all following lines to standard output.\n\n\
%s -n 1 -i - -a - -b -\n\
Copy standard input to standard output.\n\n\
\n",
program_name, program_name,
program_name);
puts("\nReturns 0 on success, and 1 on failure.\n");
printf("\nReport bugs to cs-asplit-bugs@xxxxxxxxxxxxxxx
\n\n");
}
exit(status);
}

asplit_result * asplit(const char *in_file, const char *out_file_a,
const char *out_file_b,
const long int num_lines) {

long int curr_line_num;
int curr_chr;
FILE *in_f, *out_f;
asplit_result *result;

size_t size = sizeof(asplit_result);
result = malloc(size);
if (result == NULL) {
return NULL;
} else {
result->num_rem_lines = 0;
}

/* Open in_file if not "-", returning unsuccessfully if unable
to open. */
if ((in_f = open_infile(in_file)) == NULL) {
result->status = ERROR_FILE_INPUT;
return result;
}

/* If num_lines is 0, then nothing goes into the first file.*/
if (num_lines == 0) {
/*
* So open second file if not "-", returning
unsuccessfully
* if unable to open.
*/
if ((out_f = open_outfile(out_file_b)) == NULL) {
close_file(in_f);
result->status = ERROR_FILE_OUTPUT_B;
return result;
}
} else {
/*
* If num_lines is greater than 0, then open first
file, returning
* unsuccessfully if unable to open.
*/
if ((out_f = open_outfile(out_file_a)) == NULL) {
close_file(in_f);
result->status = ERROR_FILE_OUTPUT_A;
return result;
}
}

/* Initial line number is 0, and increments after each newline
is read. */
curr_line_num = 0;

/* While input file still has characters: */
while ((curr_chr = getc(in_f)) != EOF) {

/* Output char to current output file. */
putc(curr_chr, out_f);

/*
* If we read a newline, and if we've just hit the
maximum
* allowed in the first file, we must close the first
file, and
* open the second file, returning unsuccessfully if
unable to open.
*/
if (curr_chr == '\n'&& ++curr_line_num == num_lines) {
close_file(out_f);
if ((out_f = open_outfile(out_file_b)) ==
NULL) {
close_file(in_f);
result->status = ERROR_FILE_OUTPUT_B;
return result;
}
}
}
/* Close input and output files if necessary. */
close_file(in_f);
close_file(out_f);

/* Return the number of lines output to the second file. */
result->status = 0;
result->num_rem_lines = (curr_line_num < num_lines) ? 0 :
(curr_line_num - num_lines);
return result;

}

int main(int argc, char **argv) {

program_name = *argv;

char *input_file = NULL;
char *output_file_a = NULL;
char *output_file_b = NULL;

int num_lines = -1;
int verbose = 0;
asplit_result *result = NULL;

int ich;

while ((ich = getopt(argc, argv, "hvi:a:b:n:")) != EOF) {
switch (ich) {
case 'h':
usage(0);
break;
case 'i':
input_file = (char *) copy_string(optarg);
break;
case 'a':
output_file_a = (char *) copy_string(optarg);
break;
case 'b':
output_file_b = (char *) copy_string(optarg);
break;
case 'n':
num_lines = atoi(optarg);
if (num_lines < 0)
printf("-n arg must be at least 0, not
%d.\n", num_lines);
break;
case 'v':
verbose = 1;
break;
default:
usage(1);
break;
}
}

/* Verify that all required arguments were submitted. */
if (input_file == NULL|| output_file_a == NULL|| output_file_b
== NULL
|| num_lines < 0)
usage(1);


/* Do the actual split. */
result = asplit(input_file, output_file_a, output_file_b,
num_lines);

if (result == NULL) {
fprintf(stderr, "Error: asplit returned NULL result.");
exit(1);
}

/* Determine whether it terminated successfully, and handle
accordingly. */
switch (result->status) {
case SUCCESS:
if (verbose)
printf("Output %li lines to 2nd file.\n",
result->num_rem_lines);
exit(0);
case ERROR_FILE_INPUT:
fprintf(stderr, "Unable to open input file: %s\n",
input_file);
exit(1);
case ERROR_FILE_OUTPUT_A:
fprintf(stderr, "Unable to open output file: %s\n",
output_file_a);
exit(1);
case ERROR_FILE_OUTPUT_B:
fprintf(stderr, "Unable to open output file: %s\n",
output_file_b);
exit(1);
default:
fprintf(stderr, "Error: unexpected return value %d\n", result-
status);
exit(1);
}

}

/**
* Create a copy of the string referenced by the given pointer,
returning
* a pointer to the newly created copy, or NULL if unable to malloc
memory
* for the copy or original pointer was null or the string was empty.
*
* The client is responsible for memory management of the new string.
*/
char *copy_string(const char *str_orig) {

char *str_copy = NULL;

if (str_orig != NULL && *str_orig != 0) {
size_t size = strlen(str_orig) + 1;

str_copy = malloc(size);

if (str_copy != NULL)
memcpy(str_copy, str_orig, size);
}

return str_copy;

}

/**
* Close the file referenced by the given file pointer if it is not
one of
* the standard system files.
*
* Returns 0 if the file was actually closed, and 1 if not closed
*/
int close_file(FILE * f) {
if (f != stdin&& f != stdout&& f != stderr) {
fclose(f);
return 0;
}
return 1;
}

/**
* Open the input file (mode 'r') referenced by the given pointer,
* returning a pointer to the FILE. If fname is "-", return stdin.
*/
FILE * open_infile(const char *const fname) {
return strcmp(fname, "-") == 0 ? stdin : fopen(fname, "r");
}

/**
* Open the output file (mode 'w') referenced by the given pointer,
* returning a pointer to the FILE. If fname is "-", return stdout.
*/
FILE * open_outfile(const char *const fname) {
return strcmp(fname, "-") == 0 ? stdout : fopen(fname, "w");
}

## End asplit.c ##

I did browse the FAQ already and found a couple of things to improve,
which I already did, but any additional feedback would be appreciated.

Cheers,

calvin

.

Relevant Pages

  • Re: [Lit.] Buffer overruns
    ... to retrieve from the stack. ... itself, "aha, I need to collect an int from the stack, so that'll ... The C Standard says, in 7.1.4: ... value outside the domain of the function, or a pointer outside ...
    (sci.crypt)
  • Re: Request critique of first program
    ... Returns a pointer to an asplit_result struct (unless unable to ... Success is indicated by SUCCESS, ... const char *out_file_b, ... const long int num_lines); ...
    (comp.lang.c)
  • Re: C Test Incorrectly Uses printf() - Please Confirm
    ... The C Standard appears to be evolving, ... must include an explicit "is this pointer NULL" check? ... apparent memory location (assuming a pointer was a simple memory ... int a = 1; ...
    (comp.lang.c)
  • Re: CAN WE TYPE CAST AN INTEGER AS (VOID *)X..(Like can I return a value (void *)x)
    ... >> now suppose I have declared an integer value inside a function as int ... > there is no guarantee that an int might fit into a pointer. ... Standard knows nothing about stacks and similar real world concepts). ... Vladimir ...
    (comp.lang.c)
  • Re: Plauger, size_t and ptrdiff_t
    ... Plauger's "The Standard C Library," where he states "... ... This language would not rule out one being int ... we explicitly decided to allow some pointer differences to be ... BTW, even when you do get a ptrdiff_t overflow, on a ...
    (comp.lang.c)