On 2/27/19 2:47 AM, Michael Paquier wrote: > Hi all, > (CC-ing Joe as of dc7d70e) > > I was just looking at the offline checksum patch, and noticed some > sloppy coding in controldata_utils.c. The control file gets opened in > get_controlfile(), and if it generates an error then the file > descriptor remains open. As basic code rule in the backend we should > only use BasicOpenFile() when opening files, so I think that the issue > should be fixed as attached, even if this requires including fd.h for > the backend compilation which is kind of ugly. > > Another possibility would be to just close the file descriptor before > any error, saving appropriately errno before issuing any %m portion, > but that still does not respect the backend policy regarding open().
In fd.c I see: 8<-------------------- * AllocateFile, AllocateDir, OpenPipeStream and OpenTransientFile are * wrappers around fopen(3), opendir(3), popen(3) and open(2), * respectively. They behave like the corresponding native functions, * except that the handle is registered with the current subtransaction, * and will be automatically closed at abort. These are intended mainly * for short operations like reading a configuration file; there is a * limit on the number of files that can be opened using these functions * at any one time. * * Finally, BasicOpenFile is just a thin wrapper around open() that can * release file descriptors in use by the virtual file descriptors if * necessary. There is no automatic cleanup of file descriptors returned * by BasicOpenFile, it is solely the caller's responsibility to close * the file descriptor by calling close(2). 8<-------------------- According to that comment BasicOpenFile does not seem to solve the issue you are pointing out (leaking of file descriptor on ERROR). Perhaps OpenTransientFile() is more appropriate? I am on the road at the moment so have not looked very deeply at this yet though. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
signature.asc
Description: OpenPGP digital signature