iraf-v216 · Code · Issues (50) · Pull requests (81)

iraf.net pull request #113

Cleanup readline libraries

olebole merged 2 commits to iraf-community/iraf

---

olebole commented on 2017-11-16

This PR cleans up two issues with the readline library:

1. Remove extra (unused) readline library

The library in pkg$vocl/readline/ is identical to the one in vendor$readline/. It also remains uncompiled and not used by VOCL at all. Therefore, it can be savely removed.
VOCL uses as ECL and CL the library installed in vendor$readline/.

2. Revert changes from GNU readline 5.0

The readline code in IRAF originates in the version 5.0 of the GNU readline library. Aside from Makefile changes (to fit into the IRAF build system), there are only two changes: one whitespace-only change in input.c, and one change in readline.c which was (by rumor) thought to solve a memory leak. The latter is, however, a no-op:

#define USE_STATIC  
// ...  
  
char * readline (prompt)  
const char *prompt;  
{  
 char *value = NULL; // was not initialized with NULL before  
#ifdef USE_STATIC  
  static char result[4096];  
#endif  
// ...  
      
#ifdef USE_STATIC  
  memset (result, 0, 4096);  
  if (value) {  
    strcpy (result, value);  
    free (value);  
  }  
  return (strdup (result));  
#else  
  return (value);  
#endif  
}  

There was an initialization of value with NULL added, and the code within USE_STATIC (and the #define itself) was added.
The code takes the original value that comes in value, copies it into a static area result, and then frees the original value (which [properly] assumes that value was allocated with malloc before). The copied string is then again duplicated (which implies creating the required size with malloc), and returned.

The copied string does not differ from the original one: both are malloced and need to be freed by the caller. So, this does not fix a memory leak.

Reverting this, however, makes the directory vendor$readline/ identical to the original GNU readline (except the Makefile.in adaption, and the documentation), so that it is clear that it can be replaced by linking to the system’s readline if wanted.

I checked that this does not create an (additional) memory leak. Running vocl.e with valgrind shows in both cases (with and without this patch) identical behaviour:

HEAP SUMMARY:  
    in use at exit: 292,758 bytes in 216 blocks  
  total heap usage: 2,344 allocs, 2,128 frees, 4,188,024 bytes allocated  
  
LEAK SUMMARY:  
   definitely lost: 120,540 bytes in 20 blocks  
   indirectly lost: 0 bytes in 0 blocks  
     possibly lost: 98 bytes in 1 blocks  
   still reachable: 172,120 bytes in 195 blocks  
        suppressed: 0 bytes in 0 blocks  
Rerun with --leak-check=full to see details of leaked memory  
  
For counts of detected and suppressed errors, rerun with: -v  
ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)  

This also stays stable when I do things in cl, so I don’t see a real leak here.

@iraf if you disagree, please show an example.

---

Commits

• Remove extra (unused) readline library [375edd04]
• Revert changes from GNU readline 5.0 [033ada36]

---

Last updated on 2017-11-16