-
Notifications
You must be signed in to change notification settings - Fork 86
Well managed file descriptor and mmap-ed memory #454
Conversation
#include <memory> | ||
#include <system_error> | ||
|
||
class FDManager { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, this FDManager
class represents file descriptor itself, rather than a manager. How about renaming this class to just FD
or FileDescriptor
or something?
If you are uncomfortable using such a common name, you can also use namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, fixed.
int fd; | ||
}; | ||
|
||
class SimpleMappedMem { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since MappedMem
class seems not be used in the current code, how about removing current MappedMem
class and renaming this SimpleMappedMem
class to MappedMem
class?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- readability approval
- ownership approval
* implement FileDescriptor class, rewrite MappedMem class to RAII style
Implement class for manage file descriptor / mmap-ed memory for dlk.
Motivation and Context
In current implementation, file descriptor and mmap-ed memory are not managed, so resource leakage occurs.
This PR fixes it.
Description
To manage file descriptor, FDManager
To manage mmap-ed memory, SimpleMappedMem
I considered to use std::unique_ptr:
Type of file descriptor is not pointer, so we have to implement custom class for store file descriptor which can be compare with
std::nullptr_t
.munmap()
require mapped size, so we have to implement custom class for store pointer with mapped size.These increase implement cost.
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist: