Skip to content

Commit b543488

Browse files
sebi2k1claude
sebi2k1
and
claude
committed
Apply C++20 best practices to signals.cc
- Build: add -std=c++20 to both addon targets in binding.gyp - Remove vestigial __STDC_LIMIT_MACROS (unnecessary since C++11) - Switch to C++ headers (<cstdint>, <cstring>) and add <bit> - Convert typedef enum to enum class for ENDIANESS and SIGNAL_TYPE - Fix strict-aliasing UB in _getvalue/_setvalue: replace pointer casts with memcpy-based loads; _getvalue now takes const uint8_t* - Fix misleading _setvalue signature: data[8] -> uint8_t* (64-byte buf) - Replace u_int*_t (POSIX-only) with uint*_t (ISO C++) throughout - Add [[nodiscard]] to _getvalue - _parse_signal_type: pass env, throw TypeError on unknown value instead of silently falling back to unsigned; callers check IsExceptionPending - Replace memcpy type-punning with std::bit_cast for float/double - Replace C-style casts with static_cast<> throughout - Fix signed decode: int32_t -> int64_t; use XOR-subtract sign-extension to correctly handle signals wider than 32 bits without UB Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 78760ff commit b543488

2 files changed

Lines changed: 85 additions & 116 deletions

File tree

‎binding.gyp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,17 @@
66
"include_dirs": [
77
"<!@(node -p \"require('node-addon-api').include_dir\")"
88
],
9-
"defines": [ "NAPI_DISABLE_CPP_EXCEPTIONS" ]
9+
"defines": [ "NAPI_DISABLE_CPP_EXCEPTIONS" ],
10+
"cflags_cc": [ "-std=c++20" ]
1011
},
1112
{
1213
"target_name": "can_signals",
1314
"sources": [ "native/signals.cc" ],
1415
"include_dirs": [
1516
"<!@(node -p \"require('node-addon-api').include_dir\")"
1617
],
17-
"defines": [ "NAPI_DISABLE_CPP_EXCEPTIONS" ]
18+
"defines": [ "NAPI_DISABLE_CPP_EXCEPTIONS" ],
19+
"cflags_cc": [ "-std=c++20" ]
1820
}
1921
]
2022
}

‎native/signals.cc‎

Lines changed: 81 additions & 114 deletions
Original file line numberDiff line numberDiff line change
@@ -13,92 +13,77 @@
1313
* You should have received a copy of the GNU Lesser General Public License
1414
* along with this program. If not, see <http://www.gnu.org/licenses/>.
1515
*/
16-
#define __STDC_LIMIT_MACROS
17-
1816
#include <napi.h>
1917

2018
#include <algorithm>
21-
22-
#include <stdint.h>
23-
#include <string.h>
19+
#include <bit>
20+
#include <cstdint>
21+
#include <cstring>
2422

2523
#define CHECK_CONDITION(expr, str) \
2624
if (!(expr)) { \
2725
Napi::TypeError::New(env, str).ThrowAsJavaScriptException(); \
2826
return env.Undefined(); \
2927
}
3028

31-
typedef enum ENDIANESS
29+
enum class ENDIANESS
3230
{
33-
ENDIANESS_MOTOROLA = 0,
34-
ENDIANESS_INTEL
35-
} ENDIANESS;
31+
MOTOROLA = 0,
32+
INTEL
33+
};
3634

37-
typedef enum SIGNAL_TYPE
35+
enum class SIGNAL_TYPE
3836
{
39-
SIGNAL_TYPE_UNSIGNED = 0,
40-
SIGNAL_TYPE_SIGNED = 1,
41-
SIGNAL_TYPE_FLOAT32 = 2,
42-
SIGNAL_TYPE_FLOAT64 = 3
43-
} SIGNAL_TYPE;
37+
UNSIGNED = 0,
38+
SIGNED = 1,
39+
FLOAT32 = 2,
40+
FLOAT64 = 3
41+
};
4442

45-
static SIGNAL_TYPE _parse_signal_type(const Napi::Value& v)
43+
static SIGNAL_TYPE _parse_signal_type(Napi::Env env, const Napi::Value& v)
4644
{
4745
if (v.IsBoolean())
48-
return v.As<Napi::Boolean>().Value() ? SIGNAL_TYPE_SIGNED : SIGNAL_TYPE_UNSIGNED;
46+
return v.As<Napi::Boolean>().Value() ? SIGNAL_TYPE::SIGNED : SIGNAL_TYPE::UNSIGNED;
4947
uint32_t n = v.As<Napi::Number>().Uint32Value();
50-
if (n > SIGNAL_TYPE_FLOAT64)
51-
return SIGNAL_TYPE_UNSIGNED; // safe fallback for unknown values
52-
return (SIGNAL_TYPE)n;
48+
if (n > static_cast<uint32_t>(SIGNAL_TYPE::FLOAT64)) {
49+
Napi::TypeError::New(env, "Unknown signal type").ThrowAsJavaScriptException();
50+
return SIGNAL_TYPE::UNSIGNED;
51+
}
52+
return static_cast<SIGNAL_TYPE>(n);
5353
}
5454

5555
// Returns the fixed IEEE-754 bit width for float signal types; 0 for integer types.
5656
static uint32_t signal_type_bit_width(SIGNAL_TYPE t)
5757
{
5858
switch (t) {
59-
case SIGNAL_TYPE_FLOAT32: return 32;
60-
case SIGNAL_TYPE_FLOAT64: return 64;
61-
default: return 0;
59+
case SIGNAL_TYPE::FLOAT32: return 32;
60+
case SIGNAL_TYPE::FLOAT64: return 64;
61+
default: return 0;
6262
}
6363
}
6464

6565
//-----------------------------------------------------------------------------------------
6666
// _signals.* methods
6767

68-
static u_int64_t _getvalue(u_int8_t * data,
69-
u_int32_t offset,
70-
u_int32_t length,
71-
ENDIANESS byteOrder)
68+
[[nodiscard]] static uint64_t _getvalue(const uint8_t* data,
69+
uint32_t offset,
70+
uint32_t length,
71+
ENDIANESS byteOrder)
7272
{
73-
uint64_t d;
74-
uint64_t o = 0;
75-
76-
if (byteOrder == ENDIANESS_INTEL) {
77-
d = le64toh(*((uint64_t *)&data[0]));
78-
} else {
79-
d = be64toh(*((uint64_t *)&data[0]));
80-
}
73+
uint64_t d_raw;
74+
std::memcpy(&d_raw, data, sizeof(d_raw));
75+
uint64_t d = (byteOrder == ENDIANESS::INTEL) ? le64toh(d_raw) : be64toh(d_raw);
8176

82-
uint64_t m;
83-
if (length == 64) {
84-
m = (uint64_t) UINT64_MAX;
85-
} else {
86-
m = (1LLU << length) - 1;
87-
}
88-
size_t shift;
89-
if (byteOrder == ENDIANESS_INTEL) {
90-
shift = offset;
91-
} else {
92-
shift = 64 - offset - length;
93-
}
77+
uint64_t m = (length == 64) ? UINT64_MAX : (UINT64_C(1) << length) - 1;
78+
size_t shift = (byteOrder == ENDIANESS::INTEL) ? offset : (64 - offset - length);
9479

95-
o = (d >> shift) & m;
80+
uint64_t o = (d >> shift) & m;
9681

9782
#ifdef KAYAK_DATA_CHECK
9883
size_t i;
9984
int bitNr;
10085
uint64_t val = 0;
101-
if (byteOrder == ENDIANESS_INTEL) {
86+
if (byteOrder == ENDIANESS::INTEL) {
10287
for (i = 0; i < length; i++) {
10388
bitNr = i + offset;
10489
val |= ((data[bitNr >> 3] >> (bitNr & 0x07)) & 1) << i;
@@ -142,8 +127,9 @@ Napi::Value DecodeSignal(const Napi::CallbackInfo& info)
142127

143128
offset = info[1].As<Napi::Number>().Uint32Value();
144129
bitLength = info[2].As<Napi::Number>().Uint32Value();
145-
endianess = info[3].As<Napi::Boolean>().Value() ? ENDIANESS_INTEL : ENDIANESS_MOTOROLA;
146-
SIGNAL_TYPE signalType = _parse_signal_type(info[4]);
130+
endianess = info[3].As<Napi::Boolean>().Value() ? ENDIANESS::INTEL : ENDIANESS::MOTOROLA;
131+
SIGNAL_TYPE signalType = _parse_signal_type(env, info[4]);
132+
if (env.IsExceptionPending()) return env.Undefined();
147133

148134
// Float types have a fixed width dictated by the type, not the caller.
149135
// Using signal_type_bit_width() ensures encode and decode always agree.
@@ -152,78 +138,60 @@ Napi::Value DecodeSignal(const Napi::CallbackInfo& info)
152138

153139
size_t maxBytes = std::min<size_t>(jsData.ByteLength(), sizeof(data));
154140

155-
memset(data, 0, sizeof(data));
156-
memcpy(data, jsData.Data(), maxBytes);
141+
std::memset(data, 0, sizeof(data));
142+
std::memcpy(data, jsData.Data(), maxBytes);
157143

158144
uint64_t val = _getvalue(data, offset, effectiveBitLength, endianess);
159145

160146
Napi::Value retval;
161-
if (signalType == SIGNAL_TYPE_FLOAT32) {
147+
if (signalType == SIGNAL_TYPE::FLOAT32) {
162148
// Reinterpret the 32 raw bits as IEEE-754 single precision.
163-
// _getvalue already applied the correct byte order, so a plain
164-
// memcpy into a float gives the right value on any host endianness.
165-
uint32_t raw = (uint32_t)val;
166-
float f;
167-
memcpy(&f, &raw, sizeof(f));
168-
retval = Napi::Number::New(env, (double)f);
169-
} else if (signalType == SIGNAL_TYPE_FLOAT64) {
170-
double d;
171-
memcpy(&d, &val, sizeof(d));
172-
retval = Napi::Number::New(env, d);
173-
} else if (signalType == SIGNAL_TYPE_SIGNED && val & (1LLU << (bitLength - 1))) {
174-
int32_t tmp = -1 * (~((UINT64_MAX << bitLength) | val) + 1);
175-
retval = Napi::Number::New(env, tmp);
149+
// _getvalue already applied the correct byte order, so std::bit_cast
150+
// gives the right value on any host endianness.
151+
retval = Napi::Number::New(env,
152+
static_cast<double>(std::bit_cast<float>(static_cast<uint32_t>(val))));
153+
} else if (signalType == SIGNAL_TYPE::FLOAT64) {
154+
retval = Napi::Number::New(env, std::bit_cast<double>(val));
155+
} else if (signalType == SIGNAL_TYPE::SIGNED && (val & (UINT64_C(1) << (bitLength - 1)))) {
156+
// Sign-extend from bitLength bits to int64_t.
157+
// XOR-subtract avoids UB that a naive left-shift approach would have.
158+
uint64_t sign_mask = UINT64_C(1) << (bitLength - 1);
159+
int64_t tmp = static_cast<int64_t>((val ^ sign_mask) - sign_mask);
160+
retval = Napi::Number::New(env, static_cast<double>(tmp));
176161
} else {
177-
retval = Napi::Number::New(env, (uint32_t)val);
162+
retval = Napi::Number::New(env, static_cast<uint32_t>(val));
178163
}
179164

180165
Napi::Array raw_values = Napi::Array::New(env, 2);
181166
raw_values.Set(0u, retval);
182167
// For float types the full value is in index 0; index 1 is always 0.
183-
uint32_t hi = (width > 0) ? 0u : (uint32_t)(val >> 32);
168+
uint32_t hi = (width > 0) ? 0u : static_cast<uint32_t>(val >> 32);
184169
raw_values.Set(1u, Napi::Number::New(env, hi));
185170

186171
return raw_values;
187172
}
188173

189-
void _setvalue(u_int32_t offset, u_int32_t bitLength, ENDIANESS endianess, u_int8_t data[8], u_int64_t raw_value)
174+
static void _setvalue(uint32_t offset, uint32_t bitLength, ENDIANESS endianess,
175+
uint8_t* data, uint64_t raw_value)
190176
{
191-
uint64_t o;
192-
if (endianess == ENDIANESS_INTEL) {
193-
o = le64toh(*((uint64_t *)&data[0]));
194-
} else {
195-
o = be64toh(*((uint64_t *)&data[0]));
196-
}
197-
198-
uint64_t m = 0;
199-
if (bitLength == 64) {
200-
m = (uint64_t) UINT64_MAX;
201-
} else {
202-
m = (1LLU << bitLength) - 1;
203-
}
204-
size_t shift;
205-
if (endianess == ENDIANESS_INTEL) {
206-
shift = offset;
207-
} else {
208-
shift = 64 - offset - bitLength;
209-
}
177+
uint64_t o_raw;
178+
std::memcpy(&o_raw, data, sizeof(o_raw));
179+
uint64_t o = (endianess == ENDIANESS::INTEL) ? le64toh(o_raw) : be64toh(o_raw);
210180

211-
o &= (uint64_t) ~(m << shift);
212-
o |= (uint64_t) (raw_value & m) << shift;
181+
uint64_t m = (bitLength == 64) ? UINT64_MAX : (UINT64_C(1) << bitLength) - 1;
182+
size_t shift = (endianess == ENDIANESS::INTEL) ? offset : (64 - offset - bitLength);
213183

214-
if (endianess == ENDIANESS_INTEL) {
215-
o = htole64(o);
216-
} else {
217-
o = htobe64(o);
218-
}
184+
o &= ~(m << shift);
185+
o |= (raw_value & m) << shift;
219186

220-
memcpy(&data[0], &o, 8);
187+
o = (endianess == ENDIANESS::INTEL) ? htole64(o) : htobe64(o);
188+
std::memcpy(data, &o, sizeof(o));
221189

222190
#ifdef KAYAK_DATA_CHECK
223191
size_t i;
224192
int bitNr;
225193
uint64_t val = 0;
226-
if (endianess == ENDIANESS_INTEL) {
194+
if (endianess == ENDIANESS::INTEL) {
227195
for (i = 0; i < bitLength; i++) {
228196
bitNr = i + offset;
229197
val |= ((data[bitNr >> 3] >> (bitNr & 0x07)) & 1) << i;
@@ -267,31 +235,30 @@ Napi::Value EncodeSignal(const Napi::CallbackInfo& info)
267235

268236
offset = info[1].As<Napi::Number>().Uint32Value();
269237
bitLength = info[2].As<Napi::Number>().Uint32Value();
270-
endianess = info[3].As<Napi::Boolean>().Value() ? ENDIANESS_INTEL : ENDIANESS_MOTOROLA;
271-
SIGNAL_TYPE signalType = _parse_signal_type(info[4]);
238+
endianess = info[3].As<Napi::Boolean>().Value() ? ENDIANESS::INTEL : ENDIANESS::MOTOROLA;
239+
SIGNAL_TYPE signalType = _parse_signal_type(env, info[4]);
240+
if (env.IsExceptionPending()) return env.Undefined();
272241

273242
size_t maxBytes = std::min<size_t>(jsData.ByteLength(), sizeof(data));
274-
memcpy(data, jsData.Data(), maxBytes);
275-
276-
if (signalType == SIGNAL_TYPE_FLOAT32) {
277-
float f = (float)info[5].As<Napi::Number>().DoubleValue();
278-
uint32_t raw;
279-
memcpy(&raw, &f, sizeof(raw));
280-
_setvalue(offset, signal_type_bit_width(SIGNAL_TYPE_FLOAT32), endianess, data, (uint64_t)raw);
281-
} else if (signalType == SIGNAL_TYPE_FLOAT64) {
243+
std::memcpy(data, jsData.Data(), maxBytes);
244+
245+
if (signalType == SIGNAL_TYPE::FLOAT32) {
246+
float f = static_cast<float>(info[5].As<Napi::Number>().DoubleValue());
247+
_setvalue(offset, signal_type_bit_width(SIGNAL_TYPE::FLOAT32), endianess, data,
248+
static_cast<uint64_t>(std::bit_cast<uint32_t>(f)));
249+
} else if (signalType == SIGNAL_TYPE::FLOAT64) {
282250
double d = info[5].As<Napi::Number>().DoubleValue();
283-
uint64_t raw;
284-
memcpy(&raw, &d, sizeof(raw));
285-
_setvalue(offset, signal_type_bit_width(SIGNAL_TYPE_FLOAT64), endianess, data, raw);
251+
_setvalue(offset, signal_type_bit_width(SIGNAL_TYPE::FLOAT64), endianess, data,
252+
std::bit_cast<uint64_t>(d));
286253
} else {
287-
uint64_t raw_value = info[5].As<Napi::Number>().Uint32Value();
254+
uint64_t raw_value = static_cast<uint64_t>(info[5].As<Napi::Number>().Uint32Value());
288255
if (info.Length() > 6 && (info[6].IsNumber() || info[6].IsBoolean())) {
289-
raw_value += ((uint64_t)info[6].As<Napi::Number>().Uint32Value()) << 32;
256+
raw_value += static_cast<uint64_t>(info[6].As<Napi::Number>().Uint32Value()) << 32;
290257
}
291258
_setvalue(offset, bitLength, endianess, data, raw_value);
292259
}
293260

294-
memcpy(jsData.Data(), data, maxBytes);
261+
std::memcpy(jsData.Data(), data, maxBytes);
295262

296263
return env.Undefined();
297264
}

0 commit comments

Comments
 (0)