Skip to content

Commit 954cbbe

Browse files
simzy39simzy39
authored
util/chd: use modern C++ standard library facilities (#15889)
This updates a few older parts of the CHD code to use standard C++ library features. Changes: use .data() (C++11) when passing the start of a container use std::sort (C++98) instead of qsort use std::bit_width (C++20) instead of the custom bits_for_value function use std::endian (C++20) instead of manually checking the system's byte order (I checked the std::endian replacements against the old byte-order detection: the old native_endian == 0x100 check corresponds to big-endian, and native_endian == 1 corresponds to little-endian) add or remove includes as needed for these changes * util/chd: use data() instead of &container[0] Replaces &container[0] with container.data() where a pointer to the start of a standard library container is required. Uses that refer to an offset within a container are left unchanged. This makes the intent clearer and avoids indexing the first element just to obtain the underlying data pointer. No functional change intended. AI assistance: GPT-5.6 Sol (OpenAI) was used to help identify applicable instances and review the changes. Grok 4.5 was used as an additional code review. * Remove unused metadata_hash_compare function declaration * Refactor sorting and remove unused comparison function util/chd: replace qsort with std::sort for metadata hashes Add operator< to metadata_hash and use std::sort instead of the C qsort + function pointer. Remove the now-unused metadata_hash_compare helper. * Remove cstdlib include from chd.cpp Removed unnecessary inclusion of cstdlib header, no longer needed after removing qsort * util/chd: make metadata hash comparison local to sort Move the metadata_hash comparison from operator< to a local std::sort lambda, keeping the comparison logic at the point where it is used. * util/chd: use data() for vector metadata input Use vector::data() when passing the metadata input buffer, matching the same container access cleanup in chd.cpp. * util/chd: replace bits_for_value with std::bit_width Use the C++20 std::bit_width function instead of the custom bits_for_value implementation. * Remove static method bits_for_value from chd.h Remove the now-unused bits_for_value declaration from chd.h after replacing the implementation with std::bit_width. * Fix formatting of sort function for hasharray * util/chdcodec: use data() for vector buffers Use data() when passing the start of vector buffers * tools/chdman: use data() for vector buffers Use data() when passing the start of vector buffers * util/chdcodec: use std::endian for native byte order Use std::endian instead of manually detecting native byte order
1 parent 6382245 commit 954cbbe

4 files changed

Lines changed: 66 additions & 103 deletions

File tree

‎src/lib/util/chd.cpp‎

Lines changed: 47 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020
#include <zlib.h>
2121

2222
#include <algorithm>
23+
#include <bit>
2324
#include <cassert>
2425
#include <cstddef>
25-
#include <cstdlib>
2626
#include <cstring>
2727
#include <ctime>
2828
#include <new>
@@ -251,23 +251,6 @@ inline uint64_t chd_file::file_append(const void *source, uint32_t length, uint3
251251
}
252252

253253

254-
//-------------------------------------------------
255-
// bits_for_value - return the number of bits
256-
// necessary to represent all numbers 0..value
257-
//-------------------------------------------------
258-
259-
inline uint8_t chd_file::bits_for_value(uint64_t value) noexcept
260-
{
261-
uint8_t result = 0;
262-
while (value != 0)
263-
{
264-
value >>= 1;
265-
result++;
266-
}
267-
return result;
268-
}
269-
270-
271254

272255
//**************************************************************************
273256
// CHD FILE MANAGEMENT
@@ -919,10 +902,10 @@ std::error_condition chd_file::codec_process_hunk(uint32_t hunknum)
919902
case V34_MAP_ENTRY_TYPE_COMPRESSED:
920903
{
921904
uint32_t const blocklen = get_u16be(&rawmap[12]) | (uint32_t(rawmap[14]) << 16);
922-
std::error_condition err = file_read(blockoffs, &m_compressed[0], blocklen);
905+
std::error_condition err = file_read(blockoffs, m_compressed.data(), blocklen);
923906
if (UNEXPECTED(err))
924907
return err;
925-
m_decompressor[0]->process(&m_compressed[0], blocklen);
908+
m_decompressor[0]->process(m_compressed.data(), blocklen);
926909
return std::error_condition();
927910
}
928911

@@ -958,11 +941,11 @@ std::error_condition chd_file::codec_process_hunk(uint32_t hunknum)
958941
case COMPRESSION_TYPE_2:
959942
case COMPRESSION_TYPE_3:
960943
{
961-
std::error_condition err = file_read(blockoffs, &m_compressed[0], blocklen);
944+
std::error_condition err = file_read(blockoffs, m_compressed.data(), blocklen);
962945
if (UNEXPECTED(err))
963946
return err;
964947
auto &decompressor = *m_decompressor[rawmap[0]];
965-
decompressor.process(&m_compressed[0], blocklen);
948+
decompressor.process(m_compressed.data(), blocklen);
966949
return std::error_condition();
967950
}
968951

@@ -1045,10 +1028,10 @@ std::error_condition chd_file::read_hunk(uint32_t hunknum, void *buffer)
10451028
case V34_MAP_ENTRY_TYPE_COMPRESSED:
10461029
{
10471030
uint32_t const blocklen = get_u16be(&rawmap[12]) | (uint32_t(rawmap[14]) << 16);
1048-
std::error_condition err = file_read(blockoffs, &m_compressed[0], blocklen);
1031+
std::error_condition err = file_read(blockoffs, m_compressed.data(), blocklen);
10491032
if (UNEXPECTED(err))
10501033
return err;
1051-
m_decompressor[0]->decompress(&m_compressed[0], blocklen, dest, m_hunkbytes);
1034+
m_decompressor[0]->decompress(m_compressed.data(), blocklen, dest, m_hunkbytes);
10521035
if (UNEXPECTED(!nocrc && (util::crc32_creator::simple(dest, m_hunkbytes) != blockcrc)))
10531036
return std::error_condition(error::DECOMPRESSION_ERROR);
10541037
return std::error_condition();
@@ -1115,14 +1098,14 @@ std::error_condition chd_file::read_hunk(uint32_t hunknum, void *buffer)
11151098
case COMPRESSION_TYPE_2:
11161099
case COMPRESSION_TYPE_3:
11171100
{
1118-
std::error_condition err = file_read(blockoffs, &m_compressed[0], blocklen);
1101+
std::error_condition err = file_read(blockoffs, m_compressed.data(), blocklen);
11191102
if (UNEXPECTED(err))
11201103
return err;
11211104
auto &decompressor = *m_decompressor[rawmap[0]];
1122-
decompressor.decompress(&m_compressed[0], blocklen, dest, m_hunkbytes);
1105+
decompressor.decompress(m_compressed.data(), blocklen, dest, m_hunkbytes);
11231106
util::crc16_t const calculated = !decompressor.lossy()
11241107
? util::crc16_creator::simple(dest, m_hunkbytes)
1125-
: util::crc16_creator::simple(&m_compressed[0], blocklen);
1108+
: util::crc16_creator::simple(m_compressed.data(), blocklen);
11261109
if (UNEXPECTED(calculated != blockcrc))
11271110
return std::error_condition(error::DECOMPRESSION_ERROR);
11281111
return std::error_condition();
@@ -1240,8 +1223,8 @@ std::error_condition chd_file::write_hunk(uint32_t hunknum, const void *buffer)
12401223
return err;
12411224

12421225
// update the cached hunk if we just wrote it
1243-
if (hunknum == m_cachehunk && buffer != &m_cache[0])
1244-
memcpy(&m_cache[0], buffer, m_hunkbytes);
1226+
if (hunknum == m_cachehunk && buffer != m_cache.data())
1227+
memcpy(m_cache.data(), buffer, m_hunkbytes);
12451228

12461229
return std::error_condition();
12471230
}
@@ -1329,7 +1312,7 @@ std::error_condition chd_file::read_bytes(uint64_t offset, void *buffer, uint32_
13291312
// otherwise, read from the cache
13301313
if (curhunk != m_cachehunk)
13311314
{
1332-
std::error_condition err = read_hunk(curhunk, &m_cache[0]);
1315+
std::error_condition err = read_hunk(curhunk, m_cache.data());
13331316
if (UNEXPECTED(err))
13341317
return err;
13351318
m_cachehunk = curhunk;
@@ -1381,13 +1364,13 @@ std::error_condition chd_file::write_bytes(uint64_t offset, const void *buffer,
13811364
// otherwise, write from the cache
13821365
if (curhunk != m_cachehunk)
13831366
{
1384-
err = read_hunk(curhunk, &m_cache[0]);
1367+
err = read_hunk(curhunk, m_cache.data());
13851368
if (UNEXPECTED(err))
13861369
return err;
13871370
m_cachehunk = curhunk;
13881371
}
13891372
memcpy(&m_cache[startoffs], source, endoffs + 1 - startoffs);
1390-
err = write_hunk(curhunk, &m_cache[0]);
1373+
err = write_hunk(curhunk, m_cache.data());
13911374
}
13921375

13931376
// handle errors and advance
@@ -1425,7 +1408,7 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_
14251408
// read the metadata
14261409
try { output.assign(metaentry.length, '\0'); }
14271410
catch (std::bad_alloc const &) { return std::errc::not_enough_memory; }
1428-
return file_read(metaentry.offset + METADATA_HEADER_SIZE, &output[0], metaentry.length);
1411+
return file_read(metaentry.offset + METADATA_HEADER_SIZE, output.data(), metaentry.length);
14291412
}
14301413

14311414
/**
@@ -1453,7 +1436,7 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_
14531436
// read the metadata
14541437
try { output.resize(metaentry.length); }
14551438
catch (std::bad_alloc const &) { return std::errc::not_enough_memory; }
1456-
return file_read(metaentry.offset + METADATA_HEADER_SIZE, &output[0], metaentry.length);
1439+
return file_read(metaentry.offset + METADATA_HEADER_SIZE, output.data(), metaentry.length);
14571440
}
14581441

14591442
/**
@@ -1515,7 +1498,7 @@ std::error_condition chd_file::read_metadata(chd_metadata_tag searchtag, uint32_
15151498
// read the metadata
15161499
try { output.resize(metaentry.length); }
15171500
catch (std::bad_alloc const &) { return std::errc::not_enough_memory; }
1518-
err = file_read(metaentry.offset + METADATA_HEADER_SIZE, &output[0], metaentry.length);
1501+
err = file_read(metaentry.offset + METADATA_HEADER_SIZE, output.data(), metaentry.length);
15191502
if (UNEXPECTED(err))
15201503
return err;
15211504
resulttag = metaentry.metatag;
@@ -1678,12 +1661,12 @@ std::error_condition chd_file::clone_all_metadata(chd_file &source)
16781661
// read the metadata item
16791662
try { filedata.resize(metaentry.length); }
16801663
catch (std::bad_alloc const &) { return std::errc::not_enough_memory; }
1681-
err = source.file_read(metaentry.offset + METADATA_HEADER_SIZE, &filedata[0], metaentry.length);
1664+
err = source.file_read(metaentry.offset + METADATA_HEADER_SIZE, filedata.data(), metaentry.length);
16821665
if (UNEXPECTED(err))
16831666
return err;
16841667

16851668
// write it to the destination
1686-
err = write_metadata(metaentry.metatag, (uint32_t)-1, &filedata[0], metaentry.length, metaentry.flags);
1669+
err = write_metadata(metaentry.metatag, (uint32_t)-1, filedata.data(), metaentry.length, metaentry.flags);
16871670
if (UNEXPECTED(err))
16881671
return err;
16891672
}
@@ -1725,28 +1708,34 @@ util::sha1_t chd_file::compute_overall_sha1(util::sha1_t rawsha1)
17251708

17261709
// allocate memory and read the data
17271710
filedata.resize(metaentry.length);
1728-
err = file_read(metaentry.offset + METADATA_HEADER_SIZE, &filedata[0], metaentry.length);
1711+
err = file_read(metaentry.offset + METADATA_HEADER_SIZE, filedata.data(), metaentry.length);
17291712
if (UNEXPECTED(err))
17301713
throw err;
17311714

17321715
// create an entry for this metadata and add it
17331716
metadata_hash hashentry;
17341717
put_u32be(hashentry.tag, metaentry.metatag);
1735-
hashentry.sha1 = util::sha1_creator::simple(&filedata[0], metaentry.length);
1718+
hashentry.sha1 = util::sha1_creator::simple(filedata.data(), metaentry.length);
17361719
hasharray.push_back(hashentry);
17371720
}
17381721
if (err != error::METADATA_NOT_FOUND)
17391722
throw err;
17401723

17411724
// sort the array
17421725
if (!hasharray.empty())
1743-
qsort(&hasharray[0], hasharray.size(), sizeof(hasharray[0]), metadata_hash_compare);
1726+
std::sort(
1727+
hasharray.begin(),
1728+
hasharray.end(),
1729+
[] (metadata_hash const &a, metadata_hash const &b)
1730+
{
1731+
return memcmp(&a, &b, sizeof(metadata_hash)) < 0;
1732+
});
17441733

17451734
// read the raw data hash from our header and start a new SHA1 with that data
17461735
util::sha1_creator overall_sha1;
17471736
overall_sha1.append(&rawsha1, sizeof(rawsha1));
17481737
if (!hasharray.empty())
1749-
overall_sha1.append(&hasharray[0], hasharray.size() * sizeof(hasharray[0]));
1738+
overall_sha1.append(hasharray.data(), hasharray.size() * sizeof(hasharray[0]));
17501739
return overall_sha1.finish();
17511740
}
17521741

@@ -2073,11 +2062,11 @@ std::error_condition chd_file::compress_v5_map()
20732062
try
20742063
{
20752064
// first get a CRC-16 of the original rawmap
2076-
util::crc16_t mapcrc = util::crc16_creator::simple(&m_rawmap[0], m_hunkcount * 12);
2065+
util::crc16_t mapcrc = util::crc16_creator::simple(m_rawmap.data(), m_hunkcount * 12);
20772066

20782067
// create a buffer to hold the RLE data
20792068
std::vector<uint8_t> compression_rle(m_hunkcount);
2080-
uint8_t *dest = &compression_rle[0];
2069+
uint8_t *dest = compression_rle.data();
20812070

20822071
// use a huffman encoder for 16 different codes, maximum length is 8 bits
20832072
huffman_encoder<16, 8> encoder;
@@ -2158,9 +2147,9 @@ std::error_condition chd_file::compress_v5_map()
21582147
}
21592148

21602149
// determine the number of bits we need to hold the a length and a hunk index
2161-
const uint8_t lengthbits = bits_for_value(max_complen);
2162-
const uint8_t selfbits = bits_for_value(max_self);
2163-
const uint8_t parentbits = bits_for_value(max_parent);
2150+
const uint8_t lengthbits = std::bit_width(max_complen);
2151+
const uint8_t selfbits = std::bit_width(max_self);
2152+
const uint8_t parentbits = std::bit_width(max_parent);
21642153

21652154
// determine the needed size of the output buffer
21662155
// 16 bytes is required for the header
@@ -2185,13 +2174,13 @@ std::error_condition chd_file::compress_v5_map()
21852174
throw std::error_condition(error::COMPRESSION_ERROR);
21862175

21872176
// encode the data
2188-
for (uint8_t *src = &compression_rle[0]; src < dest; src++)
2177+
for (uint8_t *src = compression_rle.data(); src < dest; src++)
21892178
encoder.encode_one(bitbuf, *src);
21902179

21912180
// for each compression type, output the relevant data
21922181
lastcomp = 0;
21932182
count = 0;
2194-
uint8_t *src = &compression_rle[0];
2183+
uint8_t *src = compression_rle.data();
21952184
uint64_t firstoffs = 0;
21962185
for (uint32_t hunknum = 0; hunknum < m_hunkcount; hunknum++)
21972186
{
@@ -2256,7 +2245,7 @@ std::error_condition chd_file::compress_v5_map()
22562245
// write the map header
22572246
uint32_t complen = bitbuf.flush();
22582247
assert(!bitbuf.overflow());
2259-
put_u32be(&compressed[0], complen);
2248+
put_u32be(compressed.data(), complen);
22602249
put_u48be(&compressed[4], firstoffs);
22612250
put_u16be(&compressed[10], mapcrc);
22622251
compressed[12] = lengthbits;
@@ -2265,7 +2254,7 @@ std::error_condition chd_file::compress_v5_map()
22652254
compressed[15] = 0;
22662255

22672256
// write the result
2268-
m_mapoffset = file_append(&compressed[0], complen + 16);
2257+
m_mapoffset = file_append(compressed.data(), complen + 16);
22692258

22702259
// then write the map offset
22712260
uint8_t rawbuf[sizeof(uint64_t)];
@@ -2298,7 +2287,7 @@ void chd_file::decompress_v5_map()
22982287
// if no offset, we haven't written it yet
22992288
if (m_mapoffset == 0)
23002289
{
2301-
memset(&m_rawmap[0], 0xff, m_rawmap.size());
2290+
memset(m_rawmap.data(), 0xff, m_rawmap.size());
23022291
return;
23032292
}
23042293

@@ -2317,10 +2306,10 @@ void chd_file::decompress_v5_map()
23172306

23182307
// now read the map
23192308
std::vector<uint8_t> compressed(mapbytes);
2320-
ioerr = file_read(m_mapoffset + 16, &compressed[0], mapbytes);
2309+
ioerr = file_read(m_mapoffset + 16, compressed.data(), mapbytes);
23212310
if (UNEXPECTED(ioerr))
23222311
throw ioerr;
2323-
bitstream_in bitbuf(&compressed[0], compressed.size());
2312+
bitstream_in bitbuf(compressed.data(), compressed.size());
23242313

23252314
// first decode the compression types
23262315
huffman_decoder<16, 8> decoder;
@@ -2409,7 +2398,7 @@ void chd_file::decompress_v5_map()
24092398
}
24102399

24112400
// verify the final CRC
2412-
if (UNEXPECTED(util::crc16_creator::simple(&m_rawmap[0], m_hunkcount * 12) != mapcrc))
2401+
if (UNEXPECTED(util::crc16_creator::simple(m_rawmap.data(), m_hunkcount * 12) != mapcrc))
24132402
throw std::error_condition(error::DECOMPRESSION_ERROR);
24142403
}
24152404

@@ -2643,7 +2632,7 @@ void chd_file::create_open_common()
26432632
}
26442633
else
26452634
{
2646-
std::error_condition err = file_read(m_mapoffset, &m_rawmap[0], m_rawmap.size());
2635+
std::error_condition err = file_read(m_mapoffset, m_rawmap.data(), m_rawmap.size());
26472636
if (UNEXPECTED(err))
26482637
throw err;
26492638
}
@@ -2906,23 +2895,6 @@ void chd_file::metadata_update_hash()
29062895
throw err;
29072896
}
29082897

2909-
/**
2910-
* @fn int CLIB_DECL chd_file::metadata_hash_compare(const void *elem1, const void *elem2)
2911-
*
2912-
* @brief -------------------------------------------------
2913-
* metadata_hash_compare - compare two hash entries
2914-
* -------------------------------------------------.
2915-
*
2916-
* @param elem1 The first element.
2917-
* @param elem2 The second element.
2918-
*
2919-
* @return A CLIB_DECL.
2920-
*/
2921-
2922-
int CLIB_DECL chd_file::metadata_hash_compare(const void *elem1, const void *elem2)
2923-
{
2924-
return memcmp(elem1, elem2, sizeof(metadata_hash));
2925-
}
29262898

29272899

29282900

@@ -3002,7 +2974,7 @@ void chd_file_compressor::compress_begin()
30022974

30032975
// reset work item state
30042976
m_work_buffer.resize(hunk_bytes() * (WORK_BUFFER_HUNKS + 1));
3005-
memset(&m_work_buffer[0], 0, m_work_buffer.size());
2977+
memset(m_work_buffer.data(), 0, m_work_buffer.size());
30062978
m_compressed_buffer.resize(hunk_bytes() * WORK_BUFFER_HUNKS);
30072979
for (int itemnum = 0; itemnum < WORK_BUFFER_HUNKS; itemnum++)
30082980
{
@@ -3306,8 +3278,8 @@ void chd_file_compressor::async_read()
33063278
if ((m_read_done_offset + numbytes) > logical_bytes())
33073279
numbytes = logical_bytes() - m_read_done_offset;
33083280

3309-
uint8_t *const dest = &m_work_buffer[0] + (m_read_done_offset % work_buffer_bytes);
3310-
assert((&m_work_buffer[0] == dest) || (&m_work_buffer[work_buffer_bytes / 2] == dest));
3281+
uint8_t *const dest = m_work_buffer.data() + (m_read_done_offset % work_buffer_bytes);
3282+
assert((m_work_buffer.data() == dest) || (&m_work_buffer[work_buffer_bytes / 2] == dest));
33113283
assert(!(m_read_done_offset % hunk_bytes()));
33123284
uint64_t const end_offset = m_read_done_offset + numbytes;
33133285

‎src/lib/util/chd.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ class chd_file
349349
std::error_condition read_metadata(chd_metadata_tag searchtag, uint32_t searchindex, std::vector<uint8_t> &output, chd_metadata_tag &resulttag, uint8_t &resultflags);
350350
std::error_condition write_metadata(chd_metadata_tag metatag, uint32_t metaindex, const void *inputbuf, uint32_t inputlen, uint8_t flags = CHD_MDFLAGS_CHECKSUM);
351351
std::error_condition write_metadata(chd_metadata_tag metatag, uint32_t metaindex, const std::string &input, uint8_t flags = CHD_MDFLAGS_CHECKSUM) { return write_metadata(metatag, metaindex, input.c_str(), input.length() + 1, flags); }
352-
std::error_condition write_metadata(chd_metadata_tag metatag, uint32_t metaindex, const std::vector<uint8_t> &input, uint8_t flags = CHD_MDFLAGS_CHECKSUM) { return write_metadata(metatag, metaindex, &input[0], input.size(), flags); }
352+
std::error_condition write_metadata(chd_metadata_tag metatag, uint32_t metaindex, const std::vector<uint8_t> &input, uint8_t flags = CHD_MDFLAGS_CHECKSUM) { return write_metadata(metatag, metaindex, input.data(), input.size(), flags); }
353353
std::error_condition delete_metadata(chd_metadata_tag metatag, uint32_t metaindex);
354354
std::error_condition clone_all_metadata(chd_file &source);
355355

@@ -376,7 +376,6 @@ class chd_file
376376
std::error_condition file_read(uint64_t offset, void *dest, uint32_t length) const noexcept;
377377
std::error_condition file_write(uint64_t offset, const void *source, uint32_t length) noexcept;
378378
uint64_t file_append(const void *source, uint32_t length, uint32_t alignment = 0);
379-
static uint8_t bits_for_value(uint64_t value) noexcept;
380379

381380
// internal helpers
382381
uint32_t guess_unitbytes();
@@ -395,7 +394,6 @@ class chd_file
395394
std::error_condition metadata_find(chd_metadata_tag metatag, int32_t metaindex, metadata_entry &metaentry, bool resume = false) const noexcept;
396395
std::error_condition metadata_set_previous_next(uint64_t prevoffset, uint64_t nextoffset) noexcept;
397396
void metadata_update_hash();
398-
static int CLIB_DECL metadata_hash_compare(const void *elem1, const void *elem2);
399397

400398
// file characteristics
401399
util::random_read_write::ptr m_file; // handle to the open core file

0 commit comments

Comments
 (0)