K&R exercise 1-19, reverse line

I've just started to read K&R and currently, I'm dealing with problem 1-19:

Write a function reverse(s) that reverses the character string s. Use it to write a program that reverses its input a line at a time.

Here is what I've implemented:

#include <stdio.h>

#define MAX_LINE_LENGTH 1000

void reverseLine(char reversedLine[], char line[], int lineSize);

int main(void) {

int c;
int i = 0;

char line[MAX_LINE_LENGTH];
while((c = getchar()) != EOF && c != '\n') {
line[i++] = c;
}
if (c == '\n')
line[i++] = '\0';

int realSize = i;
char reversedLine[realSize];

reverseLine(reversedLine, line, realSize);
printf("%s\n", reversedLine);

return 0;
}

void reverseLine(char reversedLine[], char line[], int lineSize){
int j, k;
for(j = lineSize - 2, k = 0; j >= 0; j--, k++){
reversedLine[k] = line[j];
}
reversedLine[k] = '\0';
}


I'm having problems on grasping the spirit exactly as I am supposed to be. So, any help to improve my solution will be appreciated!

• "the solution I implemented for problem 1-19" Please include a problem description. – Mast Dec 5 '17 at 14:51
• Ah, yes, my bad. Edited it now. – Said Dec 5 '17 at 16:30
• @SamOnela That's not how duplicates work on Code Review. Please read the relevant meta or find us in chat. – Mast Dec 5 '17 at 17:33
• Sorry yeah just found that... – Sᴀᴍ Onᴇᴌᴀ Dec 5 '17 at 17:34
• It looks like reversedline is allocated one character larger than it needs to be. – Snowbody Dec 5 '17 at 20:40

Good initial code.

1. No protection against buffer overflow.

char line[MAX_LINE_LENGTH];
while((c = getchar()) != EOF && c != '\n') {
if (i < MAX_LINE_LENGTH) {  // add
line[i++] = c;
}
}

2. No check for "too long a line"

// add test
if (i  == MAX_LINE_LENGTH) {
fprintf(stderr, "Input too long.\n");
return EXIT_FAILURE;
}

3. Always add the null character termination

// if (c == '\n')  // comment out line
line[i++] = '\0';

4. Code looks wrong with the - 2, yet it appears code functions correctly. The 2-ness needs explaining. Below is a size_t version. Note use of const.

void reverseLine(char reversedLine[], const char line[], size_t lineSize){
if (lineSize > 0) {
size_t lineLen = lineSize - 1;
size_t i;
for (i = 0; i < lineLen; i++) {
reversedLine[i] = line[lineLen - i];
}
reversedLine[i] = '\0';
}
}

5. Minor: for array sizing and indexing size_t is the best type. int may be too narrow in extreme cases. Yet keep in mind that size_t is some unsigned type.

6. Minor: "length" vs "size": When these two are applied to strings, using "size" to refer to the array size of the string and "length" as the value return from strlen() adds clarity. IMO, MAX_LINE_LENGTH should be LINE_SIZE or stay with MAX_LINE_LENGTH and use char line[MAX_LINE_LENGTH + 1]; ... if (i < sizeof line) { ... if (i == sizeof line) {.

void reverseLine(char reversedLine[], char line[], int lineSize);


Strictly speaking, the function, reverseLine() does not, by its declaration, indicate that reversedLine[] and lineSize[] should not overlap. It could fail in bizarre ways should they overlap.

C provides restrict to 1) indicate pointers should not point to overlapped data and 2) allow the compiler to perform additional optimizations that require that assumption.

// [] form also shifted to * form
void reverseLine(char * restrict reversedLine, const char * restrict line, int lineSize);


Alternatively code could be re-written to cope with overlapped buffers. I'll leave that as an exercise.

Design idea: In C, many of the basic string functions assume the destination array is big enough. As the standard library functions are crafted for performance, checking size allocation is left as a responsibility to the caller.

reverseLine() could be re-designed to do that test. Instead of leaving the size test to the caller, consider that reverseLine() is not a basic building block and it could check for sufficient memory. As with such things, error handling is an important, but for now, TBD consideration.

// rLineSize is the space available at rLine
// The length of the string pointed to by line can use strlen()
void reverseLine2(char * restrict rLine, size_t rLineSize, const char * restrict line) {
size_t lineSize = strlen(line) + 1;
if (lineSize > rLineSize) Handle_Error();
...

• I had no idea about that much number of missing points standing out in my code. Those are quite new and interesting stuff to me. I'll examine your comment time-to-time since I've just started to study C :) Thank you for your time and detailed explanation! – Said Dec 6 '17 at 18:03
• @SaidBuyukarslan Additional idea: Write a stand-along function for user input rather than code in main(). Modularize code for code re-use. – chux Dec 6 '17 at 18:07