patterncMinor
K&R 1-20 Converting tabs to spaces using idiomatic C
Viewed 0 times
idiomaticspacesusingtabsconverting
Problem
I'm learning C with K&R 2nd Ed. I just completed exercise 1-20 (which is the
detab program). I was hoping to get some feedback on my work. I want to make sure that I am taking a good C approach and that my knowledge in other languages isn't bleeding in./*
Exercise 1-20 in K&R 2nd Edition
detab: clarified here: http://stackoverflow.com/questions/7178201/kr-exercise-1-20-need-some-clarification
Written by Z. Bornheimer (provided as is without warranty).
*/
#include
#define MAXLEN 10000
#define TABSTOP 4
int detab(char c, char str[], int i);
/* calls detab with appropriate data */
main()
{
int i = 0;
char c, str[MAXLEN];
while ((c = getchar()) != EOF)
i = detab(c, str, i);
printf("%s\n", str);
return 0;
}
/* replaces tabs w/ spaces in accordance to TABSTOP */
int detab(char c, char str[], int i)
{
if (c == '\t')
do
str[i++] = ' ';
while ((i % TABSTOP) != 0);
else
str[i++] = c;
return i;
}Solution
I don't think this program is correct, according to my interpretation of tab stops. One problem is that the column count should reset to 0 after every carriage return or newline character encountered.
Your program is vulnerable to buffer overflow. This may be acceptable for a beginner, as long as you acknowledge the problem, but this code should never be put into production use. (You may think that your buffers are generously sized, but a malicious attacker will scoff at whatever limit you choose. There is no substitute for proper bounds checking.) One simple strategy you could use is to minimize the use of a buffer by printing output on every call to
In your
Your variables are cryptically named. While short variable names are acceptable and even encouraged as, say, dummy variables for iteration where their purpose is obvious, they should not be used as a matter of habit. In particular, it is important to give descriptive names to function parameters, since they help prevent accidental misinterpretation by the function's users. I would suggest this function interface:
For best performance, avoid reading one character at a time using
Your program is vulnerable to buffer overflow. This may be acceptable for a beginner, as long as you acknowledge the problem, but this code should never be put into production use. (You may think that your buffers are generously sized, but a malicious attacker will scoff at whatever limit you choose. There is no substitute for proper bounds checking.) One simple strategy you could use is to minimize the use of a buffer by printing output on every call to
detab() — your buffer would not need to be much larger than TABSTOP bytes.In your
detab() function, you want to treat i as an in/out parameter (i.e., the function alters the parameter and passes it back to the caller). It is customary to accomplish this in C by passing a pointer to i, like this:void func(int *in_out_param) {
while (0 != *in_out_param % 4) {
(*in_out_param)++;
}
}
void caller() {
int i = 2;
func(&i);
printf("%d\n", *i); /* prints 4 */
}Your variables are cryptically named. While short variable names are acceptable and even encouraged as, say, dummy variables for iteration where their purpose is obvious, they should not be used as a matter of habit. In particular, it is important to give descriptive names to function parameters, since they help prevent accidental misinterpretation by the function's users. I would suggest this function interface:
/**
* Converts input character c at column col into a string, with the output
* placed in buf. If c is a tab character, it is expanded into the appropriate
* number of spaces. The buffer size should be at least one more than tabwidth.
*/
void detab(unsigned int tabwidth, char c, unsigned int *col, char *buf, size_t bufsize)For best performance, avoid reading one character at a time using
getchar(). For this application, I recommend fgets(), which reads one line at a time (or up to the buffer size or until end-of-file, whichever is shortest). Admittedly, that does complicate the solution quite a bit.Code Snippets
void func(int *in_out_param) {
while (0 != *in_out_param % 4) {
(*in_out_param)++;
}
}
void caller() {
int i = 2;
func(&i);
printf("%d\n", *i); /* prints 4 */
}/**
* Converts input character c at column col into a string, with the output
* placed in buf. If c is a tab character, it is expanded into the appropriate
* number of spaces. The buffer size should be at least one more than tabwidth.
*/
void detab(unsigned int tabwidth, char c, unsigned int *col, char *buf, size_t bufsize)Context
StackExchange Code Review Q#31712, answer score: 7
Revisions (0)
No revisions yet.