vmware / splinterdb

High Performance Embedded Key-Value Store
https://splinterdb.org
Apache License 2.0
680 stars 57 forks source link

Definition of typedef bool as int32 on Linux causes integration issues w/ other s/w that define bool as BYTE. #548

Closed gapisback closed 1 year ago

gapisback commented 1 year ago

The definition of typedef int32 bool in our splinterdb/platform_linux/public_platform.h caused me a very basic integration issue with other s/w code which links with SplinterDB library. The problem is that the \<some other> code defines this as unsigned char, which is 1-byte. Splinter's definition of bool as int32 throws-off some structure layouts.

Specifically, see the layout of public splinterdb_config{} as of SHA d9fcc407 in /main, defined in include/splinterdb/platform_linux/public_platform.h:

 23 // Configuration options for SplinterDB
 24 typedef struct {
 25    // required configuration
 26    const char *filename;
 27    uint64      cache_size;
 28    uint64      disk_size;
[...]
 49    // cache
 50    bool        cache_use_stats;
 51    const char *cache_logfile;
[...]
 65    uint64 num_memtable_bg_threads;
 66    uint64 num_normal_bg_threads;
 67
 68    // btree
 69    uint64 btree_rough_count_height;
 70
 71    // filter
 72    uint64 filter_remainder_size;
 73    uint64 filter_index_size;
 74
 75    // log
 76    bool use_log;
 77
 78    // splinter
 79    uint64 memtable_capacity;

Due to the differing definitions of typedef bool, the offset of fields past the 1st instance of bool (i.e. L50 bool cache_use_stats) will differ in the .o-s compiled into the library and in the objects for the \<other code> base.

This makes the system unstable.

Asking internally with SplinterDB core-dev team, we agreed to stabilize on 1-byte bool fields.

Longer-term may be the better way would be to #include <stdbool.h> but that's not being followed through right now.

gapisback commented 1 year ago

For historical reference, this issue was aired out internally in these threads:

  1. 21.Dec.2022: Original slack post
  2. 20.Mar.2023: Follow-up post, leading to closure and creation of this issue