Timeline for get_line(): An alternative implementation of standard C library's getline() function
Current License: CC BY-SA 4.0
24 events
| when toggle format | what | by | license | comment | |
|---|---|---|---|---|---|
| Apr 17, 2022 at 8:45 | comment | added | user245050 |
@TobySpeight , there is a bug in the program but you didn't notice that. In the inner while loop (where data is being read), I am giving the address to store data as buf + index_to_load_data, but index_to_load_data is not updated in the loop. So, if read() ends up being called multiple times, then it will read new data at the same address every time, thus overwriting old data which we don't want. In code review, priority should be given to finding bugs and then to coding style.
|
|
| Mar 3, 2022 at 13:56 | comment | added | user245050 | Toby, I forgot to add the rest in that line. My complete line was this: I don't think that it is correct logic that since fds will be closed anyways (when the program exits) so its ok if they are not closed explicitly. | |
| Mar 3, 2022 at 12:01 | comment | added | Toby Speight | You specifically said, "I don't think that it is correct logic that fds will be closed anyways when the program exits." | |
| Mar 3, 2022 at 11:26 | comment | added | user245050 | Toby, I didn't say that it is not correct. I said that this should not be used as an excuse to not close fds explicitly. | |
| Feb 27, 2022 at 15:17 | comment | added | Toby Speight | You might not think that's correct, but POSIX specifies Process termination caused by any reason shall have the following consequences: … All of the file descriptors, directory streams, conversion descriptors, and message catalog descriptors open in the calling process shall be closed. Seems pretty unarguable to me. | |
| Feb 27, 2022 at 11:13 | comment | added | user245050 | Toby, I don't think that it is correct logic that fds will be closed anyways when the program exits. All programs should be correct (at least, that's my preference) - like - if you open a file then close it before exiting, if you allocate memory then free it before exiting, etc. | |
| Feb 27, 2022 at 9:47 | comment | added | Toby Speight | Arguable whether that was actually a bug, since exiting the process releases all open file descriptors anyway. I agree it's good style do explicitly close it, at least in debug builds (which you might want to test under Valgrind). | |
| Feb 27, 2022 at 6:10 | comment | added | user245050 | There was a bug in the test program but no one caught it. The fd (file) was not closed in the test program. However, I have fixed it now. | |
| Feb 25, 2022 at 15:27 | comment | added | user245050 | Richard, I meant regular files. Pipe is not a regular file. | |
| Feb 25, 2022 at 13:11 | comment | added | Richard Neumann |
@Amit That's not a bug. This program is designed to handle files only, not pipes, etc. in *nix terminology a pipe is a (kind of) file.
|
|
| Feb 25, 2022 at 12:43 | comment | added | Toby Speight | I don't know how to say it more clearly. Variables do not need to all be defined at the beginning of a function. That's what I wrote and that's what I meant. | |
| Feb 25, 2022 at 11:06 | comment | added | user245050 |
I don't understand why you wrote this: And no, C has always allowed variables to have smaller scope than an entire function. What does this mean? I thought that you are trying to say that C always allowed declare/define variables when needed. But if you were not saying that then what was the intention of this statement. I don't think we were discussing about scope of variables. We were discussing about declaring/defining variables at the top of the function vs declaring/defining variables when needed.
|
|
| Feb 25, 2022 at 10:48 | comment | added | Toby Speight |
gcc -x c -std=c89 -pedantic - <<<'int main(void) { int i=1; --i; { int j=i; return j; } }' Are you being deliberately obtuse, or do you really not understand? Either way, I don't have time for this today.
|
|
| Feb 25, 2022 at 10:44 | comment | added | user245050 |
What about this then: $ gcc -std=c89 -pedantic example.c example.c: In function ‘main’: example.c:3:46: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] . The code is int main(void) { int c = 0; printf("234\n"); int d = 0; }.
|
|
| Feb 25, 2022 at 10:36 | comment | added | Toby Speight | And no, C has always allowed variables to have smaller scope than an entire function, even back when their declarations had to be at the start of their containing block. (I'm not sure that you're in a position to claim what was "easy" for Brain and Dennis, nor to know which features were highest priority for them; for example function prototypes weren't in pre-Standard C, but we all agree they are a good thing.) | |
| Feb 25, 2022 at 10:24 | comment | added | Toby Speight | You can say to users "don't do that" - but inevitably they will. If you want them only to use regular files and not sockets or character devices, robust code will verify that and emit a meaningful diagnostic. Personally, I would find that a quite limiting restriction, but even an error message is certainly better than data corruption. | |
| Feb 25, 2022 at 9:59 | comment | added | user245050 |
Toby, another point is that if declaring/defining variables when needed is superior coding style then why didn't K & R do it from the beginning. They were very intelligent people and they could have done it easily. But they didn't do it. The only explanation for this would be that they also didn't think that declaring/defining variables when needed is superior style.
|
|
| Feb 25, 2022 at 9:57 | comment | added | user245050 |
Toby, the flexibility of declaring/defining variables when needed came with C99 in 1999. K & R was much before that. So, K & R did require that all the variables be declared/defined at the top of the function.
|
|
| Feb 25, 2022 at 9:55 | comment | added | user245050 | That's not a bug. This program is designed to handle files only, not pipes, etc. | |
| Feb 25, 2022 at 7:46 | comment | added | Toby Speight | The only bug that I spotted was the data loss when the input stream isn't seekable. | |
| Feb 25, 2022 at 7:31 | comment | added | user245050 |
Toby, you didn't give the positive review comment that you couldn't find any bugs in the program while reviewing it.
|
|
| Feb 24, 2022 at 17:50 | comment | added | Richard Neumann | @Amit We have a button for that. | |
| Feb 24, 2022 at 17:00 | comment | added | user245050 | Toby, thanks for the review. | |
| Feb 24, 2022 at 16:11 | history | answered | Toby Speight | CC BY-SA 4.0 |