# K&R Exercise 5-11. Modify the "detab" - to accept a list of tab stops as arguments

So far I have reached chapter 5 of the K&R Book, Edition 2 from which I have been learning C. I spent a few days thinking of a solution to this program. I wrote two or three versions of the same program, And the truth is that I couldn't bring myself to think of a better solution for this exercise:

Exercise 5-11. Modify the program "entab" and "detab" (written as exercises in Chapter 1.) to accept a list of tab stops as arguments. Use the default tab settings if there are no arguments.

And well, I assumed that this was the best one I had written. Here is the solution for the exercise (Chapter 5, Ex-5.11):

I would like to know how to improve it to take it "further?" or if I'm taking the right approach:

/*-
*      detab.c - expand tabs to equivalent spaces
*
*      The-C-Programming-Language Book 2nd Edition.
*
*      Created by jr.chavez on 9/22/22.
*/
#include <stdio.h>
#include <stdlib.h>

#define DTAB_STOP       8       /* DEFAULT TAB STOP */
#define MAX_TAB_STOP    100

enum escapes { BACKSPACE = '\b', TAB = '\t', NEWLINE = '\n', RETURN = '\r' };

static int
getstops(char *cp)
{
int n;

n = 0;
while (*cp) {
if (*cp < '0' || *cp > '9')
return -1;
if (*cp >= '0' && *cp <= '9')
n = n * 10 + *cp - '0';
cp++;
}
return n;
}

int
main(int argc, char *argv[])
{
int ch;
int n, col;        /* n, columns */

int nstops;
int tabstops[MAX_TAB_STOP];

nstops = argc - 1;
if (nstops >  MAX_TAB_STOP) {
fprintf(stderr, "error: too many args.\n");
exit(EXIT_FAILURE);
}
for (int indx = 1; indx < argc; indx++) {
tabstops[indx - 1] = getstops(argv[indx]) - 1;
}

col = 0;
while ((ch = getchar()) != EOF) {
switch (ch) {
case TAB:
if (nstops == 0) {
while (col < DTAB_STOP) {
printf("\u25A0");
col++;
}
continue;
}
for (n = 0; n < nstops; n++)
if (col < tabstops[n])
break;
if (n == nstops) {
printf("\u25A0");
col++;
continue;
}
while (col < tabstops[n]) {
printf("\u25A0");
col++;
}
break;
case BACKSPACE:
if (col > 0)
--col;
putchar(BACKSPACE);
break;
case RETURN:
case NEWLINE:
putchar(ch);
col = 0;
continue;
default:
putchar(ch);
break;
}
}
if (ferror(stdin)) {
perror("stdin");
return EXIT_FAILURE;
}
return 0;
}


I did some "tests":

cat test.txt | ./bin/detab 4 9 29


Input File:

#           #       #
#               #       #
#           #   #           #   #
#   #           #


Output:

#■■■■■■■■■■■■■■■■■■■■■■■■■■■■#■■#
#■■■■■■■■■■■■■■■■■■■■■■■■■■■■■#■■#
#■■■■■■■■■■■■■■■■■■■■■■■■■■■■#■#■■■#■#
#■■■#■■■■■■■■■■■■■■■■■■■■■■■■■■#


These are the results I get with the expand command: (Of course, I never intended the results to be accurate in the first place - but for now I'd just like to improve it.)

cat test.txt | expand -t 4,9,29


output:

#                            #  #
#                             #  #
#                            # #   # #
#   #                         #


Note:

• I've only written the detab program, for now.

• I had focused on doing something "similar" to what the "expand" command does. (I don't get the same output, but something similar). I also didn't add flags or read from a file. (I still haven't reached Chapter 7.).

how to improve it to take it "further?"

User input is evil.

getstops(char *cp), which should be getstops(const char *cp) does not detect invalid conversions. Better to use strtol() and test for acceptable range. Error out on bad input.

Pedantic: Hacker hardening

getstops(argv[indx]) - 1; invokes undefined behavior when getstops() returns INT_MIN. Obviously this is not an expected return value, yet better to test valid range of user values before using them in calculations.

Black square

Rather than printf("\u25A0"); consider #define BLACK_SQUARE "\u25A0", fputs(BLACK_SQUARE, stdout); or the like.

// fprintf(stderr, "error: too many args.\n");
fprintf(stderr, "error: More than %d args.\n", MAX_TAB_STOP + 1);
// or
fprintf(stderr, "error: More than %d tab stops.\n", MAX_TAB_STOP);


Good error detection

Nice: if (ferror(stdin)) {

Be generous

I could see automated tools making many tap stops.

// #define MAX_TAB_STOP    100
#define MAX_TAB_STOP    4096


Better code would simple allow argc tab stops. Size int tabstops[] to argc.

Useless code

if (*cp >= '0' && *cp <= '9') serves no purpose. It is already known cp is in range.

#include <stdlib.h>

// Parse for positive int
static int getstops(const char *cp) {
errno = 0;
char *endptr;
long val = strtol(cp, endptr, 0);
if (cp == endptr || *endptr) {
fprintf(stderr, "Non-numeric conversion <%s>\n", cp);
exit(EXIT_FAILURE); // or alternative handling
}
if (errno || val < 0 || val > INT_MAX) {
fprintf(stderr, "Invalid tab stop %ld>\n", val);
exit(EXIT_FAILURE); // or alternative handling
}
return (int) val;
}


# General Observations

Command lines may contain flags so the command line processing should check for them.

There may be more error conditions that you aren't checking for.

Some of the variables are excellent such as tabstops, variable names such as n, col and cp can be improved.

# Complexity

This was covered by part of another answer by luser droog in your last question.

The function main() is around 60 lines of code, depending on the editor or IDE you are using this may be too many lines of code. Functions that can't fit into one screen in an editor are generally considered too large. It gets very difficult to maintain a function that you can't see all at once.

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.

There are at least 3 possible functions in main().

• Process the command line
• Get the input
• In the switch statement the code to process tab should be a function.

# Use of the exit(status) Function

Inside the main() function there is no reason to call the exit() function. The exit function should only be called in functions that are not executed by the operating system.

                exit(EXIT_FAILURE);

                return EXIT_FAILURE;