I implemented the tee
command from linux. The program implements the -a
option
Reads standard input until end-of-file, write a copy of the input to standard output and to the file named in argv[1].
This is another implementation of the tee
command. I posted another code trying to implement the command, but I think I found a better implementation.
Please criticize the implementation
code:
#define _GNU_SOURCE
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#ifndef BUF_SIZE
#define BUF_SIZE 1024
#endif
static void usage(int exit_code) {
printf("Usage: mytee [-a] FILE\n");
exit(exit_code);
}
int main(int argc, char *argv[]) {
int outputFd, openFlags;
mode_t filePerms;
int aflag = 0;
char buf[BUF_SIZE + 1];
ssize_t numRead;
int c;
while ((c = getopt(argc, argv, "ha")) != -1) {
switch (c) {
case 'h': usage(0); break;
case 'a': aflag = 1; break;
}
}
if (optind >= argc) {
usage(1);
}
openFlags = O_CREAT | O_WRONLY;
if (aflag == 1) {
openFlags |= O_APPEND;
} else {
openFlags |= O_TRUNC;
}
filePerms = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP |
S_IROTH | S_IWOTH; /* rw-rw-rw- */
outputFd = open(argv[optind], openFlags, filePerms);
if (outputFd == -1) {
perror("open");
exit(1);
}
while ((numRead = read(STDIN_FILENO, buf, BUF_SIZE)) > 0) {
buf[numRead] = '0円';
// write to stdout
printf("%s", buf);
// write to output file (only up to numRead!)
if (write(outputFd, buf, numRead) != numRead) {
perror("write");
exit(1);
}
}
if (numRead == -1) {
perror("read");
exit(1);
}
if (close(outputFd) == -1) {
perror("close output file");
exit(1);
}
exit(0);
}
2 Answers 2
I see no reason to use system calls. Here it on;y hurts portability. After all, a conforming implementation is not required to provide
open, read, write
, etc. However,fopen, fread, frwite
are required, better stick tostdio
.printf("%s", buf);
immediately restricts this program to text files. Better not treatstdout
any differently from the output file. Yet again,fwrite
to both.Failing after
write(outputFd, buf, numRead) != numRead
is too pessimistic.write
is allowed to transfer fewer bytes than requested, and it is not an error. Just keep writing unless a real error (write(...) = -1
) occurs.Hardcoding the program name in a
usage
message is a poor practice. At least in Unix-like systems, the same executable may have different names (and behave differently depending on the name; for a concrete example checkvi, vim, view
). A right way is to useargv[0]
.
General Observations
The code looks like it has been copied and pasted from another implementation. One of the reasons I say this is that the exit(STATUS)
function has been called repeatedly in main(int argc, char* argv[])
. A second reason I say this is that there are only 2 functions, main()
and usage(int exit_code)
. The code within main()
is obviously broken up into logic blocks that could be functions.
The code is not portable, unistd.h
is not readily available on Windows, and types that might come from it such as mode_t
and ssize_t
don't compile on Windows. The function getopt()
is also not portable, but that could be developed as a utility/library function.
Using `exit(STATUS) in Main
There is never a good reason to call exit(STATUS)
main()
, one simply returns from main()
, exit()
only needs to be called from functions that are not the main entry point of the program. Calling exit()
in general should only be a worst case scenario, it is okay to call exit()
in an application or a shell program, but in an operating system or a device driver it is a really bad idea. The C programming language is primarily used for operating systems development, device driver development and embedded device development, calling exit()
is never a good idea in these environments so it is better not to develop the habit.
Readability and Maintainability
There are 2 standard macros defined for exit status and they are part of the C programming standard. These 2 macros are EXIT_SUCCESS
and EXIT_FAILURE
, and they are defined in stdlib.h
which is included in this program. They would make the code much more readable that exit(0)
and exit(1)
. The definitions are system dependent and that makes the code portable.
Functions / Complexity
The function main()
is too complex (does too much). As programs grow in size the use of main()
should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.
There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:
that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.
-
\$\begingroup\$ "calling exit() is never a good idea in these environments so it is better not to develop the habit" Do you expect someone who develops this habit to attempt to call
exit
from a device driver or operating system? \$\endgroup\$the default.– the default.2022年05月22日 09:41:33 +00:00Commented May 22, 2022 at 9:41 -
\$\begingroup\$ @thedefault. I've been programming in C since 1984, you might not believe all the things I've seen software engineers early in their career do. In this particular case I believe I'm dealing with someone who is trying to teach themselves how to write Unix/Linux code in C, possibly for operating systems and trying my best to teach what I know. \$\endgroup\$2022年05月22日 14:14:49 +00:00Commented May 22, 2022 at 14:14