Since this program reads its input one line at a time, I would naturally expect it to print its output line by line as well. Instead, it prints no newlines at all, which is surprising to me.
When dealing with buffers, it is usually beneficial to test whether the program works as intended with a tiny buffer. (Alternatively, you could feed it excessively long input, but reducing the buffer size is easier.) If I reduce ARRSIZE
to 10, then give it hello there
as input, I get repeated output followed by a crash:
ereht ollehereht ollehAbort trap: 6
So, you have some bugs with the if (i > ARRSIZE) printArray(reverse(buff, i), i);
logic. I'll leave it to you to figure out what went wrong.
You memset()
it to 0 after printing, but not initially. So, it looks like you are under the impression that
char buff[ARRSIZE];
gives you an array whose contents are all initially 0. However, zeroed memory is not guaranteed by the C language specification. In practice, most modern operating systems will zero all of your memory before your program runs, as a security precaution so that you can't peek at potentially sensitive junk that a previously executed program left behind in memory. However, to be strictly correct, you would only be guaranteed a blank slate if you wrote one of the following:
char buff[ARRSIZE] = { 0 };
char buff[ARRSIZE] = "";
static char buff[ARRSIZE];
I don't recommend a static
buffer for this application.
Your reverse()
function returns a static
character buffer. I don't recommend that design for two reasons:
- The buffer size is predetermined, such that it's not a general-purpose string-reversing function that works with any input.
static
buffers are error-prone designs. If you call char *a = reverse(foo)
, then char *b = reverse(bar)
, it could surprise you that a
ends up containing the reversed contents of bar
. Such side-effects are undesirable.
Based on the observation that the lengths of the input and output are the same, I would design reverse()
to accept an in-out parameter. That is, instead of returning a reversed copy of its input, it just operates on the buffer directly. Though in-out parameters may feel like an unnatural hack, they are often the best way in C to avoid memory-management headaches.
This program is supposed to process one line at a time. Therefore, the code would be clearer if it worked a line at a time rather than a character at a time. For example, you can read a whole line using fgets()
, and you can print a whole line using puts()
.
#include <stdio.h>
#include <string.h>
#define ARRSIZE 1000
/* Writes a NUL terminator at the first carriage return or newline
(if any). Returns the length of the (likely truncated) string. */
size_t chomp(char *line) {
size_t len = strcspn(line, "\r\n");
line[len] = '\0';
return len;
}
/* Reverses a buffer of len bytes in place. Returns a pointer to
the same buffer. */
char *reverse(char *buffer, size_t len) {
for (char *a = buffer, *b = buffer + len - 1; a < b; a++, b--) {
char swap = *a;
*a = *b;
*b = swap;
}
return buffer;
}
int main(void) {
char line[ARRSIZE];
while (fgets(line, sizeof line, stdin)) {
size_t len = chomp(line);
puts(reverse(line, len));
}
return ferror(stdin);
}