Changeset View
Standalone View
lib/libc/stdio/gets.c
Context not available. | |||||
__FBSDID("$FreeBSD$"); | __FBSDID("$FreeBSD$"); | ||||
#include "namespace.h" | #include "namespace.h" | ||||
#include <errno.h> | |||||
#include <unistd.h> | #include <unistd.h> | ||||
#include <stdint.h> | |||||
#include <stdio.h> | #include <stdio.h> | ||||
#include <sys/cdefs.h> | #include <sys/cdefs.h> | ||||
#include "un-namespace.h" | #include "un-namespace.h" | ||||
#include "libc_private.h" | #include "libc_private.h" | ||||
#include "local.h" | #include "local.h" | ||||
__warn_references(gets, "warning: this program uses gets(), which is unsafe."); | /* ISO/IEC 9899:2011 K.3.7.4.1 */ | ||||
ed: This comment should likely be removed, right? `_gets_s()` is not part of ISO C. | |||||
cyAuthorUnsubmitted Done Inline ActionsYes. _gets_s() is the internal function. cy: Yes. _gets_s() is the internal function. | |||||
static | |||||
char * | char * | ||||
edUnsubmitted Done Inline Actionsstatic and char * tend to be placed on a single line. ed: `static` and `char *` tend to be placed on a single line. | |||||
cyAuthorUnsubmitted Done Inline ActionsSure. cy: Sure. | |||||
gets(char *buf) | _gets_s(char *buf, rsize_t *n) | ||||
{ | { | ||||
int c; | int c; | ||||
int nn; | |||||
char *s, *ret; | char *s, *ret; | ||||
static int warned; | |||||
static const char w[] = | #define GETS_S_CALL (n != NULL) | ||||
edUnsubmitted Done Inline Actionsbool gets_s_call = n != NULL; There is no need to resort to preprocessor logic here. ed: `bool gets_s_call = n != NULL;`
There is no need to resort to preprocessor logic here. | |||||
cyAuthorUnsubmitted Done Inline ActionsAgreed. cy: Agreed. | |||||
"warning: this program uses gets(), which is unsafe.\n"; | if ( GETS_S_CALL ) { | ||||
edUnsubmitted Done Inline ActionsUnnecessary spaces inside the parentheses. ed: Unnecessary spaces inside the parentheses. | |||||
cyAuthorUnsubmitted Done Inline ActionsAgreed. cy: Agreed. | |||||
/* gets_s() */ | |||||
nn = *n; | |||||
} else { | |||||
/* Old school gets() call, not a gets_s() call. */ | |||||
/* nn could be anything, it's inconsequential at this */ | |||||
/* point. It's not used by old school gets(). This is */ | |||||
/* done to pet the compiler. */ | |||||
edUnsubmitted Done Inline ActionsMulti-line comments should be written as follows, per style(9): /* * ... * ... */ ed: Multi-line comments should be written as follows, per `style(9)`:
```
/*
* ...
* ...
*/
``` | |||||
impUnsubmitted Done Inline ActionsThis comment is incoherent and misleading. imp: This comment is incoherent and misleading.
| |||||
nn = 0; | |||||
} | |||||
FLOCKFILE_CANCELSAFE(stdin); | FLOCKFILE_CANCELSAFE(stdin); | ||||
ORIENT(stdin, -1); | ORIENT(stdin, -1); | ||||
if (!warned) { | |||||
(void) _write(STDERR_FILENO, w, sizeof(w) - 1); | for (s = buf, nn--; ((c = __sgetc(stdin)) != '\n' && | ||||
warned = 1; | ((nn > 0 && GETS_S_CALL) || ! GETS_S_CALL)); nn--) { | ||||
impUnsubmitted Done Inline ActionsWhy not just have two functions? It makes no sense at all to make them be the same if they are this different. This construct is overly complicated and once you simplify it to something sane and reasonable, you're left with so little code in common that it makes no sense to share it. The attempt to share the code makes it much worse in this case. imp: Why not just have two functions? It makes no sense at all to make them be the same if they are… | |||||
cyAuthorUnsubmitted Not Done Inline ActionsI was toying with that when I last looked at it last night. cy: I was toying with that when I last looked at it last night. | |||||
cyAuthorUnsubmitted Not Done Inline ActionsIt's been separated out. cy: It's been separated out. | |||||
} | |||||
for (s = buf; (c = __sgetc(stdin)) != '\n'; ) { | |||||
if (c == EOF) | if (c == EOF) | ||||
if (s == buf) { | if (s == buf) { | ||||
ret = NULL; | ret = NULL; | ||||
Context not available. | |||||
else | else | ||||
*s++ = c; | *s++ = c; | ||||
} | } | ||||
/****************************************************************/ | |||||
/* */ | |||||
/* If end of buffer reached, discard until \n or eof. */ | |||||
/* Then throw an error. */ | |||||
/* */ | |||||
/****************************************************************/ | |||||
if ( GETS_S_CALL && nn == 0) { | |||||
edUnsubmitted Not Done Inline ActionsThere is not a single block of code that is unaffected by GETS_S_CALL. My question then becomes, shouldn't we just make these two separate functions? ed: There is not a single block of code that is unaffected by `GETS_S_CALL`. My question then… | |||||
impUnsubmitted Not Done Inline ActionsDefinitely. imp: Definitely. | |||||
cyAuthorUnsubmitted Not Done Inline ActionsI'll rewrite it into a separate function. Additionally, I have a poudriere build that has a bit of gas with my latest version: gcc6. cy: I'll rewrite it into a separate function. Additionally, I have a poudriere build that has a bit… | |||||
/* discard */ | |||||
while (((c = __sgetc(stdin)) != '\n') && c != EOF); | |||||
/* throw the error */ | |||||
__throw_constraint_handler_s("gets_s : end of buffer", E2BIG); | |||||
return(NULL); | |||||
} | |||||
*s = 0; | *s = 0; | ||||
ret = buf; | ret = buf; | ||||
end: | end: | ||||
FUNLOCKFILE_CANCELSAFE(); | FUNLOCKFILE_CANCELSAFE(); | ||||
return (ret); | return (ret); | ||||
} | } | ||||
/* ISO/IEC 9899:2011 K.3.7.4.1 */ | |||||
char * | |||||
gets_s(char *buf, rsize_t n) | |||||
{ | |||||
if (buf == NULL) { | |||||
__throw_constraint_handler_s("gets_s : str is NULL", EINVAL); | |||||
return(NULL); | |||||
} else if (n > RSIZE_MAX) { | |||||
__throw_constraint_handler_s("gets_s : n > RSIZE_MAX", | |||||
EINVAL); | |||||
return(NULL); | |||||
} else if (n == 0) { | |||||
__throw_constraint_handler_s("gets_s : n == 0", EINVAL); | |||||
return(NULL); | |||||
} | |||||
return(_gets_s(buf, &n)); | |||||
edUnsubmitted Not Done Inline ActionsMissing space between return and ( ed: Missing space between `return` and `(` | |||||
} | |||||
#define __GETS_ENABLED | |||||
#ifdef __GETS_ENABLED | |||||
__warn_references(gets, "warning: this program uses gets(), which is unsafe."); | |||||
asomersUnsubmitted Not Done Inline ActionsHow about a consider using gets_s(3) instead. ? asomers: How about a `consider using gets_s(3) instead`. ? | |||||
cyAuthorUnsubmitted Not Done Inline ActionsThis is why I was toying with splitting this into another file. There is no way to warn about gets(3) and not have a warning with gets_s(3). This has to go into a separate file. cy: This is why I was toying with splitting this into another file. There is no way to warn about… | |||||
char * | |||||
gets(char *buf) | |||||
{ | |||||
static int warned; | |||||
static const char w[] = | |||||
"warning: this program uses gets(), which is unsafe.\n"; | |||||
if (!warned) { | |||||
(void) _write(STDERR_FILENO, w, sizeof(w) - 1); | |||||
warned = 1; | |||||
} | |||||
return(_gets_s(buf, NULL)); | |||||
} | |||||
#endif | |||||
Context not available. |
This comment should likely be removed, right? _gets_s() is not part of ISO C.