Improve style.md
This commit is contained in:
69
style.md
69
style.md
@@ -26,7 +26,7 @@ This document describes the C++ coding style used in the WeaselDB project. These
|
|||||||
- **Always use std:: prefixed versions** of C library functions for consistency and clarity
|
- **Always use std:: prefixed versions** of C library functions for consistency and clarity
|
||||||
- **Use C++ style headers** (`<cstring>`, `<cstdlib>`, etc.) instead of C style headers (`<string.h>`, `<stdlib.h>`, etc.)
|
- **Use C++ style headers** (`<cstring>`, `<cstdlib>`, etc.) instead of C style headers (`<string.h>`, `<stdlib.h>`, etc.)
|
||||||
- This applies to all standard libc functions: `std::abort()`, `std::fprintf()`, `std::free()`, `std::memcpy()`, `std::strlen()`, `std::strncpy()`, `std::memset()`, `std::signal()`, etc.
|
- This applies to all standard libc functions: `std::abort()`, `std::fprintf()`, `std::free()`, `std::memcpy()`, `std::strlen()`, `std::strncpy()`, `std::memset()`, `std::signal()`, etc.
|
||||||
- Exception: Functions with no std:: equivalent (e.g., `perror()`, `gai_strerror()`) and system-specific headers (e.g., `<unistd.h>`, `<fcntl.h>`)
|
- **Exception:** Functions with no std:: equivalent (e.g., `perror()`, `gai_strerror()`) and system-specific headers (e.g., `<unistd.h>`, `<fcntl.h>`)
|
||||||
```cpp
|
```cpp
|
||||||
// Preferred - C++ style
|
// Preferred - C++ style
|
||||||
#include <cstring>
|
#include <cstring>
|
||||||
@@ -89,7 +89,8 @@ auto ptr = static_cast<int*>(malloc(sizeof(int))); // Explicit conversion
|
|||||||
auto base = static_cast<BaseClass*>(derived_ptr); // Safe upcast
|
auto base = static_cast<BaseClass*>(derived_ptr); // Safe upcast
|
||||||
auto derived = dynamic_cast<DerivedClass*>(base_ptr); // Runtime type checking
|
auto derived = dynamic_cast<DerivedClass*>(base_ptr); // Runtime type checking
|
||||||
auto mutable_ptr = const_cast<int*>(const_ptr); // Remove const (rare)
|
auto mutable_ptr = const_cast<int*>(const_ptr); // Remove const (rare)
|
||||||
auto addr = reinterpret_cast<uintptr_t>(ptr); // Low-level operations (very rare, delicate)
|
// reinterpret_cast can be appropriate for low-level operations (very rare)
|
||||||
|
auto addr = reinterpret_cast<uintptr_t>(ptr); // Pointer to integer conversion
|
||||||
```
|
```
|
||||||
|
|
||||||
### Performance Focus
|
### Performance Focus
|
||||||
@@ -99,7 +100,7 @@ auto addr = reinterpret_cast<uintptr_t>(ptr); // Low-level operations (v
|
|||||||
- **Strive for 0% CPU usage when idle** - avoid polling, busy waiting, or unnecessary background activity
|
- **Strive for 0% CPU usage when idle** - avoid polling, busy waiting, or unnecessary background activity
|
||||||
- Use **inline functions** for performance-critical code (e.g., `allocate_raw`)
|
- Use **inline functions** for performance-critical code (e.g., `allocate_raw`)
|
||||||
- **Zero-copy operations** with `std::string_view` over string copying
|
- **Zero-copy operations** with `std::string_view` over string copying
|
||||||
- **Arena allocation** for efficient memory management (see Memory Management section for details)
|
- **Arena allocation** for efficient memory management (~1ns vs ~20-270ns for malloc)
|
||||||
|
|
||||||
### Complexity Control
|
### Complexity Control
|
||||||
- **Encapsulation is the main tool for controlling complexity**
|
- **Encapsulation is the main tool for controlling complexity**
|
||||||
@@ -124,11 +125,11 @@ void add_block(int64_t size);
|
|||||||
int32_t initial_block_size_;
|
int32_t initial_block_size_;
|
||||||
```
|
```
|
||||||
|
|
||||||
### Structs
|
### Classes and Structs
|
||||||
- **PascalCase** for struct names
|
- **PascalCase** for class/struct names
|
||||||
- **Always use struct** - eliminates debates about complexity and maintains consistency
|
- **Always use struct keyword** - eliminates debates about complexity and maintains consistency
|
||||||
- **Public members first, private after** - puts the interface users care about at the top, implementation details below
|
- **Public members first, private after** - puts the interface users care about at the top, implementation details below
|
||||||
- **Full encapsulation still applies** - use `private:` sections to hide implementation details and maintain deep, capable classes
|
- **Full encapsulation still applies** - use `private:` sections to hide implementation details and maintain deep, capable structs
|
||||||
- The struct keyword doesn't mean shallow design - it means interface-first organization for human readers
|
- The struct keyword doesn't mean shallow design - it means interface-first organization for human readers
|
||||||
```cpp
|
```cpp
|
||||||
struct ArenaAllocator {
|
struct ArenaAllocator {
|
||||||
@@ -220,8 +221,8 @@ std::unique_ptr<Parser> parser;
|
|||||||
|
|
||||||
## Code Structure
|
## Code Structure
|
||||||
|
|
||||||
### Struct Design
|
### Class Design
|
||||||
- **Move-only semantics** for resource-owning structs
|
- **Move-only semantics** for resource-owning types
|
||||||
- **Explicit constructors** to prevent implicit conversions
|
- **Explicit constructors** to prevent implicit conversions
|
||||||
- **Delete copy operations** when inappropriate
|
- **Delete copy operations** when inappropriate
|
||||||
```cpp
|
```cpp
|
||||||
@@ -229,12 +230,12 @@ struct ArenaAllocator {
|
|||||||
explicit ArenaAllocator(int64_t initial_size = 1024);
|
explicit ArenaAllocator(int64_t initial_size = 1024);
|
||||||
|
|
||||||
// Copy construction is not allowed
|
// Copy construction is not allowed
|
||||||
ArenaAllocator(const ArenaAllocator &) = delete;
|
ArenaAllocator(const ArenaAllocator &source) = delete;
|
||||||
ArenaAllocator &operator=(const ArenaAllocator &) = delete;
|
ArenaAllocator &operator=(const ArenaAllocator &source) = delete;
|
||||||
|
|
||||||
// Move semantics
|
// Move semantics
|
||||||
ArenaAllocator(ArenaAllocator &&other) noexcept;
|
ArenaAllocator(ArenaAllocator &&source) noexcept;
|
||||||
ArenaAllocator &operator=(ArenaAllocator &&other) noexcept;
|
ArenaAllocator &operator=(ArenaAllocator &&source) noexcept;
|
||||||
|
|
||||||
private:
|
private:
|
||||||
int32_t initial_block_size_;
|
int32_t initial_block_size_;
|
||||||
@@ -251,9 +252,9 @@ private:
|
|||||||
- **noexcept specification** for move operations and non-throwing functions
|
- **noexcept specification** for move operations and non-throwing functions
|
||||||
```cpp
|
```cpp
|
||||||
std::span<const Operation> operations() const { return operations_; }
|
std::span<const Operation> operations() const { return operations_; }
|
||||||
void process_data(std::string_view data); // ≤ 16 bytes, pass by value
|
void process_data(std::string_view request_data); // ≤ 16 bytes, pass by value
|
||||||
void process_request(const CommitRequest& req); // > 16 bytes, pass by reference
|
void process_request(const CommitRequest& commit_request); // > 16 bytes, pass by reference
|
||||||
ArenaAllocator(ArenaAllocator &&other) noexcept;
|
ArenaAllocator(ArenaAllocator &&source) noexcept;
|
||||||
```
|
```
|
||||||
|
|
||||||
### Template Usage
|
### Template Usage
|
||||||
@@ -273,13 +274,15 @@ ArenaAllocator(ArenaAllocator &&other) noexcept;
|
|||||||
auto server = Server::create(config, handler); // Returns shared_ptr
|
auto server = Server::create(config, handler); // Returns shared_ptr
|
||||||
|
|
||||||
// Exclusive ownership - single owner, transfer via move
|
// Exclusive ownership - single owner, transfer via move
|
||||||
auto connection = Connection::createForServer(...); // Returns unique_ptr
|
auto connection = Connection::createForServer(addr, fd, connection_id, handler, server_ref);
|
||||||
|
|
||||||
// Friend-based factory for access control
|
// Friend-based factory for access control
|
||||||
struct Connection {
|
struct Connection {
|
||||||
void appendMessage(std::string_view data);
|
void appendMessage(std::string_view message_data);
|
||||||
private:
|
private:
|
||||||
Connection(/* args */); // Private constructor
|
Connection(struct sockaddr_storage client_addr, int file_descriptor,
|
||||||
|
int64_t connection_id, ConnectionHandler* request_handler,
|
||||||
|
std::weak_ptr<Server> server_ref);
|
||||||
friend struct Server; // Only Server can construct
|
friend struct Server; // Only Server can construct
|
||||||
};
|
};
|
||||||
```
|
```
|
||||||
@@ -302,15 +305,15 @@ for (auto &precondition : preconditions_) {
|
|||||||
## Memory Management
|
## Memory Management
|
||||||
|
|
||||||
### Ownership & Allocation
|
### Ownership & Allocation
|
||||||
- **Arena allocators** for request-scoped memory with **STL allocator adapters** (provides ~1ns allocation vs ~20-270ns for malloc)
|
- **Arena allocators** for request-scoped memory with **STL allocator adapters** (see Performance Focus section for characteristics)
|
||||||
- **String views** pointing to arena-allocated memory for zero-copy operations
|
- **String views** pointing to arena-allocated memory for zero-copy operations
|
||||||
- **STL containers with arena allocators require default construction after arena reset** - `clear()` is not sufficient
|
- **STL containers with arena allocators require default construction after arena reset** - `clear()` is not sufficient
|
||||||
```cpp
|
```cpp
|
||||||
// STL containers with arena allocators - correct reset pattern
|
// STL containers with arena allocators - correct reset pattern
|
||||||
std::vector<Operation, ArenaStlAllocator<Operation>> operations(arena_alloc);
|
std::vector<Operation, ArenaStlAllocator<Operation>> operations(arena_allocator);
|
||||||
// ... use container ...
|
// ... use container ...
|
||||||
operations = {}; // Default construct - clear() won't work correctly
|
operations = {}; // Default construct - clear() won't work correctly
|
||||||
arena.reset(); // Reset arena memory
|
arena_allocator.reset(); // Reset arena memory
|
||||||
```
|
```
|
||||||
|
|
||||||
### Resource Management
|
### Resource Management
|
||||||
@@ -346,12 +349,12 @@ arena.reset(); // Reset arena memory
|
|||||||
enum class ParseResult { Success, InvalidJson, MissingField };
|
enum class ParseResult { Success, InvalidJson, MissingField };
|
||||||
|
|
||||||
// System failure - abort immediately
|
// System failure - abort immediately
|
||||||
void* memory = malloc(size);
|
void* memory = std::malloc(size);
|
||||||
if (!memory) {
|
if (!memory) {
|
||||||
std::fprintf(stderr, "ArenaAllocator: Memory allocation failed\n");
|
std::fprintf(stderr, "ArenaAllocator: Memory allocation failed\n");
|
||||||
std::abort();
|
std::abort();
|
||||||
}
|
}
|
||||||
// ... use memory, eventually free it
|
// ... use memory, eventually std::free(memory)
|
||||||
|
|
||||||
// Programming error - precondition violation (may be omitted for performance)
|
// Programming error - precondition violation (may be omitted for performance)
|
||||||
assert(ptr != nullptr && "Precondition violated: pointer must be non-null");
|
assert(ptr != nullptr && "Precondition violated: pointer must be non-null");
|
||||||
@@ -396,9 +399,9 @@ do {
|
|||||||
} while (fd == -1 && errno == EINTR);
|
} while (fd == -1 && errno == EINTR);
|
||||||
|
|
||||||
if (fd == -1) {
|
if (fd == -1) {
|
||||||
// Handle other errors
|
// Handle other errors (perror has no std:: equivalent)
|
||||||
perror("accept");
|
perror("accept");
|
||||||
abort();
|
std::abort();
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -408,9 +411,9 @@ The `close()` system call is a special case on Linux. According to `man 2 close`
|
|||||||
|
|
||||||
```cpp
|
```cpp
|
||||||
// Correct: Do not retry close() on EINTR
|
// Correct: Do not retry close() on EINTR
|
||||||
int e = close(fd);
|
int result = close(fd);
|
||||||
if (e == -1 && errno != EINTR) {
|
if (result == -1 && errno != EINTR) {
|
||||||
// Handle non-EINTR errors only
|
// Handle non-EINTR errors only (perror has no std:: equivalent)
|
||||||
perror("close");
|
perror("close");
|
||||||
std::abort();
|
std::abort();
|
||||||
}
|
}
|
||||||
@@ -444,14 +447,14 @@ When in doubt, consult the `man` page for the specific system call to see if it
|
|||||||
```cpp
|
```cpp
|
||||||
/**
|
/**
|
||||||
* @brief Type-safe version of realloc_raw for arrays of type T.
|
* @brief Type-safe version of realloc_raw for arrays of type T.
|
||||||
* @param ptr Pointer to the existing allocation
|
* @param existing_ptr Pointer to the existing allocation
|
||||||
* @param old_size Size in number of T objects
|
* @param current_size Size in number of T objects
|
||||||
* @param new_size Desired new size in number of T objects
|
* @param requested_size Desired new size in number of T objects
|
||||||
* @return Pointer to reallocated memory
|
* @return Pointer to reallocated memory
|
||||||
* @note Prints error to stderr and calls std::abort() if allocation fails
|
* @note Prints error to stderr and calls std::abort() if allocation fails
|
||||||
*/
|
*/
|
||||||
template <typename T>
|
template <typename T>
|
||||||
T *realloc(T *ptr, int32_t old_size, int32_t new_size);
|
T *realloc(T *existing_ptr, int32_t current_size, int32_t requested_size);
|
||||||
```
|
```
|
||||||
|
|
||||||
### Code Comments
|
### Code Comments
|
||||||
|
|||||||
Reference in New Issue
Block a user