Avoid use of gets() in DOE-ProxyApps-C/Pathfinder

[arichardson]

Summary

gets() has been removed from C11 and on FreeBSD 13 libc no longer provides
it by default. Use fgets() instead.

[jrtc27]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

gets always removes the newline, but fgets will include it (except in the case where the input reaches EOF before one is found) so the final character needs to be removed if it's \n.

[arichardson]

remove newline

[krytarowski]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

Can you introduce mygets that implements gets_s internally?

[arichardson]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

I could do, but that seems like overkill for the one use of gets().

[krytarowski]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

static char *mygets(char *str, size_t n)
{
    char *ret;
    size_t last;

    if ((ret = fgets(str, n, stdin)) != NULL) {
        last = strlen(str) - 1;

        if (str[last] == '\n' || str[last] == '\r' )
            str[last] = '\0';
    }

    return ret;
}

And then use mygets(stringBuffer, sizeof(stringBuffer));

It looks nicer to me.

[arichardson]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

That will not work with zero-length strings (it will access str[-1]),

Adding stringBuffer[strcspn(stringBuffer, "\r\n")] = '\0'; seems much simpler to me.

[jrtc27]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

Also the \r check is misguided. In text mode, line endings get normalised to just \n. In binary mode they will stay as-is, but on Windows that means you have \r\n (and \r on classic Mac OS 9 and below I guess) so the if would need to be a while and last decremented. But stdin should be in text mode by default.

[arichardson]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

strcspn() returns the length up to the first occurrence of \r or \n, so it should work?

[jrtc27]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

Yeah, that avoids any such complications. Technically the \r can just be dropped from it I believe but it does no harm (well unless people feed in \r on systems with Unix line endings I guess...).

[krytarowski]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

That will not work with zero-length strings (it will access str[-1]),

How so? fgets returns NULL for 0-length strings.

I find this switch more elegant with gets_s()-like code, but this is a matter of taste.

[arichardson]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

I'd prefer gets_s, but we can't assume that the platform provides it. using fgets+ strcspn is only one additional line and supported everywhere so it's IMO the simpliest solution. If there were more uses of gets(), then a wrapper might make sense.

[krytarowski]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

316

I'd prefer gets_s, but we can't assume that the platform provides it.

As I proposed, including it inline as mygets or similar, as almost no platforms delivers it.

using fgets+ strcspn is only one additional line and supported everywhere so it's IMO the simpliest solution.

In my personal taste open-coding this looks awful, but this is up to you.

If the LOC would matter in my proposal, mygets could be compressed to 2 lines too.

If there were more uses of gets(), then a wrapper might make sense.

This is fine.

[dim]

Add mygets() and implement it hopefully to everybody's liking.

[jrtc27]

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

290

1. len >= 1, no? Otherwise a blank line will not have a newline stripped.
2. This doesn't handle \r\n correctly; either assume stdin is in text mode so you only ever see \n or find the first instance of one of the characters (either with strcspn or by a loop that walks backwards here)

[dim]

Assume stdin is in text mode and compare correctly.

[arichardson]

LGTM.

MultiSource/Benchmarks/DOE-ProxyApps-C/Pathfinder/main.c

void runBatch(char *configFile, Configuration *config)
     #else
         doMultiSearches(config);
     #endif
     }
 
     printf ("\n\nSearches complete.\n");
 }
 
+void mygets(char *str, size_t n)
+{
+    size_t len;
+
+    if ( fgets(str, n, stdin) == NULL )
+    {
+        printf("Error: No input provided.\n\n\t---Exiting---");
+        exit(0);
+    }
+
+    len = strlen(str);
+    if ( len > 1 && (str[len - 1] == '\n' || str[len - 1] == '\r') )
+        str[len - 1] = '\0';
+}
 
 void runInteractively(char *fileName, Configuration *config)
 {
     char stringBuffer[200];
     Graph *graph;
     char *labels[25]; /* Do we need an interactive signature bigger than 25? */
     NodePtrVec *path = NULL;
     bool success = false;
     int i, j;
     int fullIntSignature[3] = { 0, 0, -1 };
     
 
     graph = parseFile(fileName);
 
     if ( !graph )
         return;
 
     while ( 1 )
     {
         i = 0;
         do {
 
             printf("\nPlease insert a node label for this signature (\"\" to complete, \"bail\" to exit):\n");
-            gets(stringBuffer);
+            mygets(stringBuffer, sizeof stringBuffer);
 
             if ( strcmp (stringBuffer, "bail") == 0 )
                 exit(1);
 
             if ( strcmp(stringBuffer, "") != 0 )
             {
                 labels[i] = strdup(stringBuffer);
                 ++i;