replace gets by fgets

[pdebuyl]

add function get_input that relies on fgets to accept input up to a fixed length.

In this PR, I tried a minimal revision of example.c to remove gets. Every call to gets is replaced by in = get_input() (in being now a char *) and a check for in==NULL.

I tried to make get_input robust to overlong line (shows "Entry too long" and clears input buffer).

If the idea of a minimal rewrite is not good, as you hinted at in #228 , don't bother commenting in detail, I would give it a try with a different type of program.

[Quuxplusone]

I kind of like this direction, but get_input is way too heavyweight IMO. You could simplify it a lot by just aborting on invalid input — which is still a step up from gets having UB on invalid input, right? Something like

const char *get_input() {
    static char buf[100];
    printf("99 char max> "); fflush(stdout);
    char *p = fgets(buf, sizeof(buf), stdin);
    if (p == NULL || (p = strchr(buf, '\n')) == NULL) {
        puts("Invalid input!");
        exit(EXIT_FAILURE);
    }
    *p = '\0';
    return buf;
}

and then remove all the error-checking branches from main. Try that.

[pdebuyl]

Hi, thanks for the feedback.

I used your version of get_input and made the following changes

• used const qualifier for variable to add_user
• use named constant for buffer length
• register delete_all with atexit to avoid leaks on failure

[pdebuyl]

PS: feel free to squash the commits

[Quuxplusone]

Okay, I've got a candidate patch here: https://github.com/Quuxplusone/uthash/commit/a109c6b702b42cb5e134f489bb48a2b5aaa88ade I'm trying to keep example.c short, so I'm not going to worry about letting people redefine the buffer size nor making sure everything gets free'd on abnormal exit. You see any major problems with a109c6b702b42cb5e134f489bb48a2b5aaa88ade ?

[pdebuyl]

Hi @Quuxplusone the patch is fine by me. Some of my motivation was to keep valgrind from finding leaks, but example.c is not supposed to be used as a component by users :-)

There is one remaining unrelated bug: using "rename/add by id" leads to "id" not being increased. Line 109, changing add_user(id, getl("Name (20 char max)")); to add_user(id++, getl("Name (20 char max)")); would fix that.

[Quuxplusone]

There is one remaining unrelated bug: using "rename/add by id" leads to "id" not being increased. Line 109, changing add_user(id, getl("Name (20 char max)")); to add_user(id++, getl("Name (20 char max)")); would fix that.

Oh yeah; ick. I'm pretty sure the intent there was that "rename/add by id" shouldn't disturb the increasing sequence of ids used by "add", so really it shouldn't be assigning into id at all; it should either use a separate variable instead of id, or figure out a way to avoid using a variable at all. I'll think about it.

[Quuxplusone]

Merged a109c6b702b42cb5e134f489bb48a2b5aaa88ade — thanks! And merged bf15263081be6229be31addd48566df93921cb46 to deal with the "unrelated bug".

[pdebuyl]

Thanks for considering the changes.