Project

General

Profile

Bug #1717

Broken error handling in function gmx_tmpnam

Added by Olivier Fisette over 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
core library
Target version:
Affected version - extra info:
probably every version since 37b450f33
Affected version:
Difficulty:
uncategorized
Close

Description

The "gmx_tmpnam" function used for temporary file creation does error handling by comparing the file descriptor returned by the C library function "mkstemp" to some of the possible codes that are actually stored in the "errno" global variable. This will not catch any error, and will cause a spurious error when the file handle happens to have the same value as one of the error codes. I noticed this because it broke "gmx do_dssp" given some combination of compiler and Gromacs versions; "do_dssp" would then complain "Permission denied" when the file was actually created without problem.

IO error handling in C should generally be done by checking the validity of the file descriptor. The error code can then be used to provide an informative message. I attach a patch that fixes "gmx_tmpnam", but I have not checked if other functions are affected.

gmx_tmpnam-error-handling.patch View (752 Bytes) Olivier Fisette, 04/21/2015 12:57 PM

Associated revisions

Revision 95c7de01 (diff)
Added by Olivier Fisette over 2 years ago

Fix error handling in gmx_tmpnam

The return code of mkstemp was being mis-used for error handling.
This could explain some long-standing issues with (e.g.) DSSP
mysteriously not working even when the user had done everything right.

Fixes #1717

Change-Id: I72b385a751b99c3f49d99a14bfc6964ad776c22d

History

#1 Updated by Gerrit Code Review Bot over 2 years ago

Gerrit received a related patchset '1' for Issue #1717.
Uploader: Mark Abraham ()
Change-Id: I72b385a751b99c3f49d99a14bfc6964ad776c22d
Gerrit URL: https://gerrit.gromacs.org/4531

#2 Updated by Mark Abraham over 2 years ago

  • Category set to core library
  • Status changed from New to Fix uploaded
  • Assignee set to Mark Abraham
  • Target version set to 5.0.5
  • Affected version - extra info set to probably every version since 37b450f33

Thanks, good catch! I uploaded your patch for code review at the above link.

#3 Updated by Mark Abraham over 2 years ago

  • Status changed from Fix uploaded to Accepted
  • Target version changed from 5.0.5 to 5.1

Issue fixed for 5.0.5, but improvements to windows support are suggested in the gerrit discussion.

#4 Updated by Erik Lindahl about 2 years ago

  • Status changed from Accepted to Fix uploaded

#5 Updated by Erik Lindahl about 2 years ago

  • Status changed from Fix uploaded to Resolved

#6 Updated by Erik Lindahl about 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF