threads / patch / 62969

v6, 18 partsreftable/stack: stop using `read_in_full()`

Subject: [PATCH v6 01/18] reftable/stack: stop using `read_in_full()`

## tl;dr

23 messages between Feb 18, 2025 and Mar 20, 2025. Diffs are folded; open one to read it.

replies: 22people: 2as markdown or json

Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · lore

[PATCH v6 00/18] reftable: stop using "git-compat-util.h"

Hi,

this patch series is the final step to fully decouple the reftable library from the rest of the Git codebase. The goal of this is to make the library reusable by other projects like libgit2 by simply copying over the source files, making Git the canonical upstream for reftable functionality.

This patch series stops using all kinds of helpers exposed by our "git-compat-util.h" header and open-codes them instead. In order to keep us from using these helpers by accident the final step is to pull out POSIX-related bits and pieces into a new "compat/posix.h" header, which the reftable library then uses instead of "git-compat-util.h".

Changes in v3:
  - Fix type of `total_read` variable used to track how many bytes we
    have read in `fd_read_lines()`.
  - Drop the patch use root-relative includes again. Let's rather
    discuss this outside of the scope of this series.
  - Link to v2: https://lore.kernel.org/r/20250128-pks-reftable-drop-git-compat-util-v2-0-c85c20336317@pks.im
Changes in v4:
  - Fix a couple of now-invalid relative includes that I missed. This
    fixes the build issue in "seen" with Meson.
  - Link to v3: https://lore.kernel.org/r/20250203-pks-reftable-drop-git-compat-util-v3-0-446c9ed4ee9e@pks.im
Changes in v5:
  - Rework the implementation of `REFTABLE_UNUSED` to use
    `__attribute__((__unused__))`.
  - Link to v4: https://lore.kernel.org/r/20250206-pks-reftable-drop-git-compat-util-v4-0-603d276d5f95@pks.im
Changes in v6:
  - I have rebased the patch series on top of "master" at 03944513488
    (The eleventh batch, 2025-02-14). On the one hand this gets rid of
    the conflict with the zlib-ng changes, which have meanwhile been
    merged to that branch. And on the other hand it pulls in the new CI
    build for MSVC+Meson, which allows me to have better test coverage
    on Windows to better detect breakage there.
  - Stop moving around Win32 files so that we do the minimum viable
    change in this context. Hopefully, this makes it easier for Dscho to
    integrate into Git for Windows. If it doesn't I don't really have
    any other good ideas for how to do it, so the only alternative would
    be to just ignore Windows altogether.
  - Link to v5: https://lore.kernel.org/r/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im
Thanks!
Patrick
---
Patrick Steinhardt (18):
      reftable/stack: stop using `read_in_full()`
      reftable/stack: stop using `write_in_full()`
      reftable/blocksource: stop using `xmmap()`
      reftable/record: stop using `COPY_ARRAY()`
      reftable/record: stop using `BUG()` in `reftable_record_init()`
      reftable/record: don't `BUG()` in `reftable_record_cmp()`
      reftable: stop using `BUG()` in trivial cases
      reftable/basics: stop using `st_mult()` in array allocators
      reftable/basics: provide wrappers for big endian conversion
      reftable/reader: stop using `ARRAY_SIZE()` macro
      reftable/system: introduce `reftable_rand()`
      reftable/stack: stop using `sleep_millisec()`
      reftable/basics: stop using `SWAP()` macro
      reftable/basics: introduce `REFTABLE_UNUSED` annotation
      compat/mingw: split out POSIX-related bits
      git-compat-util.h: split out POSIX-emulating bits
      reftable: decouple from Git codebase by pulling in "compat/posix.h"
      Makefile: skip reftable library for Coccinelle
 Makefile                         |   2 +-
 compat/mingw-posix.h             | 431 +++++++++++++++++++++++++++++++
 compat/mingw.h                   | 426 +-----------------------------
 compat/msvc-posix.h              |  33 +++
 compat/msvc.h                    |  30 +--
 compat/posix.h                   | 541 +++++++++++++++++++++++++++++++++++++++
 git-compat-util.h                | 531 +-------------------------------------
 reftable/basics.c                |  19 --
 reftable/basics.h                | 123 ++++++++-
 reftable/block.c                 |  16 +-
 reftable/blocksource.c           |  17 +-
 reftable/iter.c                  |  13 +-
 reftable/merged.c                |  27 +-
 reftable/pq.c                    |  40 ++-
 reftable/pq.h                    |   2 +-
 reftable/reader.c                |  33 ++-
 reftable/record.c                |  84 +++---
 reftable/record.h                |   6 +-
 reftable/stack.c                 |  52 +++-
 reftable/system.c                |   7 +
 reftable/system.h                |   8 +-
 reftable/writer.c                |  27 +-
 t/unit-tests/t-reftable-basics.c |  28 +-
 t/unit-tests/t-reftable-pq.c     |  22 +-
 t/unit-tests/t-reftable-record.c |  42 ++-
 25 files changed, 1405 insertions(+), 1155 deletions(-)
Range-diff versus v5:
 1:  18b6360460f =  1:  3589d759e69 reftable/stack: stop using `read_in_full()`
 2:  4426ec59a5e =  2:  e9fb7b0821f reftable/stack: stop using `write_in_full()`
 3:  4db9255d16d =  3:  f509993ee13 reftable/blocksource: stop using `xmmap()`
 4:  2752fb006c5 =  4:  48247ebf145 reftable/record: stop using `COPY_ARRAY()`
 5:  3bdd13f015e =  5:  795e9650146 reftable/record: stop using `BUG()` in `reftable_record_init()`
 6:  21529172e83 =  6:  8221aec91a9 reftable/record: don't `BUG()` in `reftable_record_cmp()`
 7:  e62c056f8e8 =  7:  8563af898e5 reftable: stop using `BUG()` in trivial cases
 8:  b506a862abc =  8:  447c7166c97 reftable/basics: stop using `st_mult()` in array allocators
 9:  b1749f88610 =  9:  108d40a63f5 reftable/basics: provide wrappers for big endian conversion
10:  f583f0cfb03 = 10:  19068751b6c reftable/reader: stop using `ARRAY_SIZE()` macro
11:  c59328f9299 ! 11:  316ca16a18c reftable/system: introduce `reftable_rand()`
    @@ reftable/system.c
     
      ## reftable/system.h ##
     @@ reftable/system.h: license that can be found in the LICENSE file or at
    - 
      #include "git-compat-util.h"
    + #include "compat/zlib-compat.h"
      
     +/*
     + * Return a random 32 bit integer. This function is expected to return
12:  0657d144327 = 12:  3cf2df99203 reftable/stack: stop using `sleep_millisec()`
13:  504ae579c30 = 13:  5184d4185a6 reftable/basics: stop using `SWAP()` macro
14:  34b0ae5f697 = 14:  a108a876c9a reftable/basics: introduce `REFTABLE_UNUSED` annotation
15:  1552c1f45bf <  -:  ----------- compat/mingw: split out POSIX-related bits
 -:  ----------- > 15:  48186eb7aeb compat/mingw: split out POSIX-related bits
16:  7cebd3c3381 ! 16:  16f2371c868 git-compat-util.h: split out POSIX-emulating bits
    @@ compat/posix.h (new)
     +#endif
     +
     +#if defined(__MINGW32__)
    -+#include "mingw/posix.h"
    ++#include "mingw-posix.h"
     +#elif defined(_MSC_VER)
    -+#include "msvc/posix.h"
    ++#include "msvc-posix.h"
     +#else
     +#include <sys/utsname.h>
     +#include <sys/wait.h>
    @@ git-compat-util.h: static inline int _have_unix_sockets(void)
     @@ git-compat-util.h: static inline int is_xplatform_dir_sep(int c)
      #elif defined(_MSC_VER)
      #include "compat/win32/path-utils.h"
    - #include "compat/msvc/compat-util.h"
    + #include "compat/msvc.h"
     -#else
     -#include <sys/utsname.h>
     -#include <sys/wait.h>
17:  dbb9b816d2e ! 17:  4461c8cf4a3 reftable: decouple from Git codebase by pulling in "compat/posix.h"
    @@ reftable/system.h: license that can be found in the LICENSE file or at
      /* This header glues the reftable library to the rest of Git */
      
     -#include "git-compat-util.h"
    -+#include "../compat/posix.h"
    -+#include <zlib.h>
    ++#include "compat/posix.h"
    + #include "compat/zlib-compat.h"
      
      /*
    -  * Return a random 32 bit integer. This function is expected to return
18:  f7b470f3387 = 18:  3e70da1abc8 Makefile: skip reftable library for Coccinelle

--- base-commit: 03944513488db4a81fdb4c21c3b515e4cb260b05 change-id: 20241119-pks-reftable-drop-git-compat-util-470f2bfde562

Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

There is a single callsite of `read_in_full()` in the reftable library. Open-code the function to reduce our dependency on the Git library.

Note that we only partially port over the logic from `read_in_full()` and its underlying `xread()` helper. Most importantly, the latter also knows to handle `EWOULDBLOCK` via `handle_nonblock()`. This logic is irrelevant for us though because the reftable library never sets the `O_NONBLOCK` option in the first place.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
Show changes to reftable/stack.c +14 −4
diff --git a/reftable/stack.c b/reftable/stack.c
index 6c4e8be19b1..1cc47b94d9c 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -115,13 +115,16 @@ int reftable_new_stack(struct reftable_stack **dest, const char *dir,
 
 static int fd_read_lines(int fd, char ***namesp)
 {
-	off_t size = lseek(fd, 0, SEEK_END);
 	char *buf = NULL;
 	int err = 0;
+	off_t size;
+
+	size = lseek(fd, 0, SEEK_END);
 	if (size < 0) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
+
 	err = lseek(fd, 0, SEEK_SET);
 	if (err < 0) {
 		err = REFTABLE_IO_ERROR;
@@ -134,9 +137,16 @@ static int fd_read_lines(int fd, char ***namesp)
 		goto done;
 	}
 
-	if (read_in_full(fd, buf, size) != size) {
-		err = REFTABLE_IO_ERROR;
-		goto done;
+	for (off_t total_read = 0; total_read < size; ) {
+		ssize_t bytes_read = read(fd, buf + total_read, size - total_read);
+		if (bytes_read < 0 && (errno == EAGAIN || errno == EINTR))
+			continue;
+		if (bytes_read < 0 || !bytes_read) {
+			err = REFTABLE_IO_ERROR;
+			goto done;
+		}
+
+		total_read += bytes_read;
 	}
 	buf[size] = 0;
 
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 02/18] reftable/stack: stop using `write_in_full()`

Similar to the preceding commit, drop our use of `write_in_full()` and implement a new wrapper `reftable_write_full()` that handles this logic for us. This is done to reduce our dependency on the Git library.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)
Show changes to reftable/stack.c +24 −4
diff --git a/reftable/stack.c b/reftable/stack.c
index 1cc47b94d9c..c33f0c3333a 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -48,6 +48,25 @@ static int stack_fsync(const struct reftable_write_options *opts, int fd)
 	return fsync(fd);
 }
 
+static ssize_t reftable_write_data(int fd, const void *data, size_t size)
+{
+	size_t total_written = 0;
+	const char *p = data;
+
+	while (total_written < size) {
+		ssize_t bytes_written = write(fd, p, size - total_written);
+		if (bytes_written < 0 && (errno == EAGAIN || errno == EINTR))
+			continue;
+		if (bytes_written < 0)
+			return REFTABLE_IO_ERROR;
+
+		total_written += bytes_written;
+		p += bytes_written;
+	}
+
+	return total_written;
+}
+
 struct fd_writer {
 	const struct reftable_write_options *opts;
 	int fd;
@@ -56,7 +75,7 @@ struct fd_writer {
 static ssize_t fd_writer_write(void *arg, const void *data, size_t sz)
 {
 	struct fd_writer *writer = arg;
-	return write_in_full(writer->fd, data, sz);
+	return reftable_write_data(writer->fd, data, sz);
 }
 
 static int fd_writer_flush(void *arg)
@@ -784,7 +803,8 @@ int reftable_addition_commit(struct reftable_addition *add)
 			goto done;
 	}
 
-	err = write_in_full(add->tables_list_lock.fd, table_list.buf, table_list.len);
+	err = reftable_write_data(add->tables_list_lock.fd,
+				  table_list.buf, table_list.len);
 	reftable_buf_release(&table_list);
 	if (err < 0) {
 		err = REFTABLE_IO_ERROR;
@@ -1470,8 +1490,8 @@ static int stack_compact_range(struct reftable_stack *st,
 			goto done;
 	}
 
-	err = write_in_full(tables_list_lock.fd,
-			    tables_list_buf.buf, tables_list_buf.len);
+	err = reftable_write_data(tables_list_lock.fd,
+				  tables_list_buf.buf, tables_list_buf.len);
 	if (err < 0) {
 		err = REFTABLE_IO_ERROR;
 		unlink(new_table_path.buf);
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 03/18] reftable/blocksource: stop using `xmmap()`

We use `xmmap()` to map reftables into memory. This function has two problems:

  - It causes us to die in case the mmap fails.
  - It ties us to the Git codebase.

Refactor the code to use mmap(3p) instead with manual error checking. Note that this function may not be the system-provided mmap(3p), but may point to our `git_mmap()` wrapper that emulates the syscall on systems that do not have mmap(3p) available.

Fix `reftable_block_source_from_file()` to properly bubble up the error code in case the map(3p) call fails.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/blocksource.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
Show changes to reftable/blocksource.c +8 −3
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index bba4a45b98a..02972c46f4b 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -98,7 +98,7 @@ static struct reftable_block_source_vtable file_vtable = {
 int reftable_block_source_from_file(struct reftable_block_source *bs,
 				    const char *name)
 {
-	struct file_block_source *p;
+	struct file_block_source *p = NULL;
 	struct stat st;
 	int fd, err;
 
@@ -122,7 +122,12 @@ int reftable_block_source_from_file(struct reftable_block_source *bs,
 	}
 
 	p->size = st.st_size;
-	p->data = xmmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	p->data = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
+	if (p->data == MAP_FAILED) {
+		err = REFTABLE_IO_ERROR;
+		p->data = NULL;
+		goto out;
+	}
 
 	assert(!bs->ops);
 	bs->ops = &file_vtable;
@@ -135,5 +140,5 @@ int reftable_block_source_from_file(struct reftable_block_source *bs,
 		close(fd);
 	if (err < 0)
 		reftable_free(p);
-	return 0;
+	return err;
 }
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 04/18] reftable/record: stop using `COPY_ARRAY()`

Drop our use of `COPY_ARRAY()`, replacing it with an open-coded variant thereof. This is done to reduce our dependency on the Git library.

While at it, guard the whole array copy logic so that we only copy it in case there actually is anything to be copied. Otherwise, we may end up trying to allocate a zero-sized array, which will return a NULL pointer and thus cause us to return an `REFTABLE_OUT_OF_MEMORY_ERROR`.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/record.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)
Show changes to reftable/record.c +11 −5
diff --git a/reftable/record.c b/reftable/record.c
index 8919df8a4d3..2c0cc32cbd3 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -504,11 +504,17 @@ static int reftable_obj_record_copy_from(void *rec, const void *src_rec,
 	if (src->hash_prefix_len)
 		memcpy(obj->hash_prefix, src->hash_prefix, obj->hash_prefix_len);
 
-	REFTABLE_ALLOC_ARRAY(obj->offsets, src->offset_len);
-	if (!obj->offsets)
-		return REFTABLE_OUT_OF_MEMORY_ERROR;
-	obj->offset_len = src->offset_len;
-	COPY_ARRAY(obj->offsets, src->offsets, src->offset_len);
+	if (src->offset_len) {
+		if (sizeof(*src->offsets) > SIZE_MAX / src->offset_len)
+			return REFTABLE_OUT_OF_MEMORY_ERROR;
+
+		REFTABLE_ALLOC_ARRAY(obj->offsets, src->offset_len);
+		if (!obj->offsets)
+			return REFTABLE_OUT_OF_MEMORY_ERROR;
+
+		memcpy(obj->offsets, src->offsets, sizeof(*src->offsets) * src->offset_len);
+		obj->offset_len = src->offset_len;
+	}
 
 	return 0;
 }
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 05/18] reftable/record: stop using `BUG()` in `reftable_record_init()`

We're aborting the program via `BUG()` in case `reftable_record_init()` was invoked with an unknown record type. This is bad because we may now die in library code, and because it makes us depend on the Git codebase.

Refactor the code such that `reftable_record_init()` can return an error code to the caller. Adapt any callers accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/block.c                 | 4 +++-
 reftable/merged.c                | 5 ++++-
 reftable/reader.c                | 5 ++++-
 reftable/record.c                | 8 ++++----
 reftable/record.h                | 4 ++--
 t/unit-tests/t-reftable-pq.c     | 6 +++---
 t/unit-tests/t-reftable-record.c | 2 +-
 7 files changed, 21 insertions(+), 13 deletions(-)
Show changes to 7 files +21 −13

reftable/block.c, reftable/merged.c, reftable/reader.c, reftable/record.c, reftable/record.h, t/unit-tests/t-reftable-pq.c, t/unit-tests/t-reftable-record.c

diff --git a/reftable/block.c b/reftable/block.c
index b14a8f1259a..999876826d1 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -508,7 +508,9 @@ int block_iter_seek_key(struct block_iter *it, const struct block_reader *br,
 	it->block_len = br->block_len;
 	it->hash_size = br->hash_size;
 
-	reftable_record_init(&rec, block_reader_type(br));
+	err = reftable_record_init(&rec, block_reader_type(br));
+	if (err < 0)
+		goto done;
 
 	/*
 	 * We're looking for the last entry less than the wanted key so that
diff --git a/reftable/merged.c b/reftable/merged.c
index e72b39e178d..4156eec07fc 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -253,7 +253,10 @@ int merged_table_init_iter(struct reftable_merged_table *mt,
 	}
 
 	for (size_t i = 0; i < mt->readers_len; i++) {
-		reftable_record_init(&subiters[i].rec, typ);
+		ret = reftable_record_init(&subiters[i].rec, typ);
+		if (ret < 0)
+			goto out;
+
 		ret = reader_init_iter(mt->readers[i], &subiters[i].iter, typ);
 		if (ret < 0)
 			goto out;
diff --git a/reftable/reader.c b/reftable/reader.c
index 3f2e4b28005..de6e6dd9322 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -360,7 +360,10 @@ static int table_iter_seek_linear(struct table_iter *ti,
 	struct reftable_record rec;
 	int err;
 
-	reftable_record_init(&rec, reftable_record_type(want));
+	err = reftable_record_init(&rec, reftable_record_type(want));
+	if (err < 0)
+		goto done;
+
 	err = reftable_record_key(want, &want_key);
 	if (err < 0)
 		goto done;
diff --git a/reftable/record.c b/reftable/record.c
index 2c0cc32cbd3..1e18f8dffb1 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -1306,7 +1306,7 @@ reftable_record_vtable(struct reftable_record *rec)
 	abort();
 }
 
-void reftable_record_init(struct reftable_record *rec, uint8_t typ)
+int reftable_record_init(struct reftable_record *rec, uint8_t typ)
 {
 	memset(rec, 0, sizeof(*rec));
 	rec->type = typ;
@@ -1315,11 +1315,11 @@ void reftable_record_init(struct reftable_record *rec, uint8_t typ)
 	case BLOCK_TYPE_REF:
 	case BLOCK_TYPE_LOG:
 	case BLOCK_TYPE_OBJ:
-		return;
+		return 0;
 	case BLOCK_TYPE_INDEX:
 		reftable_buf_init(&rec->u.idx.last_key);
-		return;
+		return 0;
 	default:
-		BUG("unhandled record type");
+		return REFTABLE_API_ERROR;
 	}
 }
diff --git a/reftable/record.h b/reftable/record.h
index c7755a4d750..e1846c294ba 100644
--- a/reftable/record.h
+++ b/reftable/record.h
@@ -130,8 +130,8 @@ struct reftable_record {
 	} u;
 };
 
-/* Initialize the reftable record for the given type */
-void reftable_record_init(struct reftable_record *rec, uint8_t typ);
+/* Initialize the reftable record for the given type. */
+int reftable_record_init(struct reftable_record *rec, uint8_t typ);
 
 /* see struct record_vtable */
 int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b);
diff --git a/t/unit-tests/t-reftable-pq.c b/t/unit-tests/t-reftable-pq.c
index f3f8a0cdf38..d8a4c283a11 100644
--- a/t/unit-tests/t-reftable-pq.c
+++ b/t/unit-tests/t-reftable-pq.c
@@ -32,7 +32,7 @@ static void t_pq_record(void)
 	char *last = NULL;
 
 	for (i = 0; i < N; i++) {
-		reftable_record_init(&recs[i], BLOCK_TYPE_REF);
+		check(!reftable_record_init(&recs[i], BLOCK_TYPE_REF));
 		recs[i].u.ref.refname = xstrfmt("%02"PRIuMAX, (uintmax_t)i);
 	}
 
@@ -72,7 +72,7 @@ static void t_pq_index(void)
 	size_t N = ARRAY_SIZE(recs), i;
 
 	for (i = 0; i < N; i++) {
-		reftable_record_init(&recs[i], BLOCK_TYPE_REF);
+		check(!reftable_record_init(&recs[i], BLOCK_TYPE_REF));
 		recs[i].u.ref.refname = (char *) "refs/heads/master";
 	}
 
@@ -111,7 +111,7 @@ static void t_merged_iter_pqueue_top(void)
 	size_t N = ARRAY_SIZE(recs), i;
 
 	for (i = 0; i < N; i++) {
-		reftable_record_init(&recs[i], BLOCK_TYPE_REF);
+		check(!reftable_record_init(&recs[i], BLOCK_TYPE_REF));
 		recs[i].u.ref.refname = (char *) "refs/heads/master";
 	}
 
diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
index d49d2a2729c..6540bd20e37 100644
--- a/t/unit-tests/t-reftable-record.c
+++ b/t/unit-tests/t-reftable-record.c
@@ -17,7 +17,7 @@ static void t_copy(struct reftable_record *rec)
 	uint8_t typ;
 
 	typ = reftable_record_type(rec);
-	reftable_record_init(&copy, typ);
+	check(!reftable_record_init(&copy, typ));
 	reftable_record_copy_from(&copy, rec, REFTABLE_HASH_SIZE_SHA1);
 	/* do it twice to catch memory leaks */
 	reftable_record_copy_from(&copy, rec, REFTABLE_HASH_SIZE_SHA1);
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 06/18] reftable/record: don't `BUG()` in `reftable_record_cmp()`

The reftable library aborts with a bug in case `reftable_record_cmp()` is invoked with two records of differing types. This would cause the program to die without the caller being able to handle the error, which is not something we want in the context of library code. And it ties us to the Git codebase.

Refactor the code such that `reftable_record_cmp()` returns an error code separate from the actual comparison result. This requires us to also adapt some callers up the callchain in a similar fashion.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/merged.c                | 20 +++++++++++++++-----
 reftable/pq.c                    | 36 +++++++++++++++++++++++++++++-------
 reftable/pq.h                    |  2 +-
 reftable/record.c                | 10 ++++++----
 reftable/record.h                |  2 +-
 t/unit-tests/t-reftable-pq.c     | 16 ++++++++++++----
 t/unit-tests/t-reftable-record.c | 40 ++++++++++++++++++++++++++++------------
 7 files changed, 92 insertions(+), 34 deletions(-)
Show changes to 7 files +92 −34

reftable/merged.c, reftable/pq.c, reftable/pq.h, reftable/record.c, reftable/record.h, t/unit-tests/t-reftable-pq.c, t/unit-tests/t-reftable-record.c

diff --git a/reftable/merged.c b/reftable/merged.c
index 4156eec07fc..563864068c1 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -66,8 +66,11 @@ static int merged_iter_seek(struct merged_iter *mi, struct reftable_record *want
 	int err;
 
 	mi->advance_index = -1;
-	while (!merged_iter_pqueue_is_empty(mi->pq))
-		merged_iter_pqueue_remove(&mi->pq);
+	while (!merged_iter_pqueue_is_empty(mi->pq)) {
+		err = merged_iter_pqueue_remove(&mi->pq, NULL);
+		if (err < 0)
+			return err;
+	}
 
 	for (size_t i = 0; i < mi->subiters_len; i++) {
 		err = iterator_seek(&mi->subiters[i].iter, want);
@@ -120,7 +123,9 @@ static int merged_iter_next_entry(struct merged_iter *mi,
 	if (empty)
 		return 1;
 
-	entry = merged_iter_pqueue_remove(&mi->pq);
+	err = merged_iter_pqueue_remove(&mi->pq, &entry);
+	if (err < 0)
+		return err;
 
 	/*
 	  One can also use reftable as datacenter-local storage, where the ref
@@ -134,11 +139,16 @@ static int merged_iter_next_entry(struct merged_iter *mi,
 		struct pq_entry top = merged_iter_pqueue_top(mi->pq);
 		int cmp;
 
-		cmp = reftable_record_cmp(top.rec, entry.rec);
+		err = reftable_record_cmp(top.rec, entry.rec, &cmp);
+		if (err < 0)
+			return err;
 		if (cmp > 0)
 			break;
 
-		merged_iter_pqueue_remove(&mi->pq);
+		err = merged_iter_pqueue_remove(&mi->pq, NULL);
+		if (err < 0)
+			return err;
+
 		err = merged_iter_advance_subiter(mi, top.index);
 		if (err < 0)
 			return err;
diff --git a/reftable/pq.c b/reftable/pq.c
index 5591e875e1e..ef8035cfd9b 100644
--- a/reftable/pq.c
+++ b/reftable/pq.c
@@ -15,13 +15,18 @@ license that can be found in the LICENSE file or at
 
 int pq_less(struct pq_entry *a, struct pq_entry *b)
 {
-	int cmp = reftable_record_cmp(a->rec, b->rec);
+	int cmp, err;
+
+	err = reftable_record_cmp(a->rec, b->rec, &cmp);
+	if (err < 0)
+		return err;
+
 	if (cmp == 0)
 		return a->index > b->index;
 	return cmp < 0;
 }
 
-struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq)
+int merged_iter_pqueue_remove(struct merged_iter_pqueue *pq, struct pq_entry *out)
 {
 	size_t i = 0;
 	struct pq_entry e = pq->heap[0];
@@ -32,17 +37,34 @@ struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq)
 		size_t min = i;
 		size_t j = 2 * i + 1;
 		size_t k = 2 * i + 2;
-		if (j < pq->len && pq_less(&pq->heap[j], &pq->heap[i]))
-			min = j;
-		if (k < pq->len && pq_less(&pq->heap[k], &pq->heap[min]))
-			min = k;
+		int cmp;
+
+		if (j < pq->len) {
+			cmp = pq_less(&pq->heap[j], &pq->heap[i]);
+			if (cmp < 0)
+				return -1;
+			else if (cmp)
+				min = j;
+		}
+
+		if (k < pq->len) {
+			cmp = pq_less(&pq->heap[k], &pq->heap[min]);
+			if (cmp < 0)
+				return -1;
+			else if (cmp)
+				min = k;
+		}
+
 		if (min == i)
 			break;
 		SWAP(pq->heap[i], pq->heap[min]);
 		i = min;
 	}
 
-	return e;
+	if (out)
+		*out = e;
+
+	return 0;
 }
 
 int merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e)
diff --git a/reftable/pq.h b/reftable/pq.h
index 83c062eecac..ff39016445b 100644
--- a/reftable/pq.h
+++ b/reftable/pq.h
@@ -22,7 +22,7 @@ struct merged_iter_pqueue {
 	size_t cap;
 };
 
-struct pq_entry merged_iter_pqueue_remove(struct merged_iter_pqueue *pq);
+int merged_iter_pqueue_remove(struct merged_iter_pqueue *pq, struct pq_entry *out);
 int merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry *e);
 void merged_iter_pqueue_release(struct merged_iter_pqueue *pq);
 int pq_less(struct pq_entry *a, struct pq_entry *b);
diff --git a/reftable/record.c b/reftable/record.c
index 1e18f8dffb1..b39d99fcc75 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -1195,12 +1195,14 @@ int reftable_record_is_deletion(struct reftable_record *rec)
 		reftable_record_data(rec));
 }
 
-int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b)
+int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b,
+			int *cmp)
 {
 	if (a->type != b->type)
-		BUG("cannot compare reftable records of different type");
-	return reftable_record_vtable(a)->cmp(
-		reftable_record_data(a), reftable_record_data(b));
+		return -1;
+	*cmp = reftable_record_vtable(a)->cmp(reftable_record_data(a),
+					      reftable_record_data(b));
+	return 0;
 }
 
 int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, uint32_t hash_size)
diff --git a/reftable/record.h b/reftable/record.h
index e1846c294ba..867810a9328 100644
--- a/reftable/record.h
+++ b/reftable/record.h
@@ -134,7 +134,7 @@ struct reftable_record {
 int reftable_record_init(struct reftable_record *rec, uint8_t typ);
 
 /* see struct record_vtable */
-int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b);
+int reftable_record_cmp(struct reftable_record *a, struct reftable_record *b, int *cmp);
 int reftable_record_equal(struct reftable_record *a, struct reftable_record *b, uint32_t hash_size);
 int reftable_record_key(struct reftable_record *rec, struct reftable_buf *dest);
 int reftable_record_copy_from(struct reftable_record *rec,
diff --git a/t/unit-tests/t-reftable-pq.c b/t/unit-tests/t-reftable-pq.c
index d8a4c283a11..c128fe8616a 100644
--- a/t/unit-tests/t-reftable-pq.c
+++ b/t/unit-tests/t-reftable-pq.c
@@ -21,7 +21,9 @@ static void merged_iter_pqueue_check(const struct merged_iter_pqueue *pq)
 
 static int pq_entry_equal(struct pq_entry *a, struct pq_entry *b)
 {
-	return !reftable_record_cmp(a->rec, b->rec) && (a->index == b->index);
+	int cmp;
+	check(!reftable_record_cmp(a->rec, b->rec, &cmp));
+	return !cmp && (a->index == b->index);
 }
 
 static void t_pq_record(void)
@@ -49,7 +51,9 @@ static void t_pq_record(void)
 
 	while (!merged_iter_pqueue_is_empty(pq)) {
 		struct pq_entry top = merged_iter_pqueue_top(pq);
-		struct pq_entry e = merged_iter_pqueue_remove(&pq);
+		struct pq_entry e;
+
+		check(!merged_iter_pqueue_remove(&pq, &e));
 		merged_iter_pqueue_check(&pq);
 
 		check(pq_entry_equal(&top, &e));
@@ -90,7 +94,9 @@ static void t_pq_index(void)
 
 	for (i = N - 1; i > 0; i--) {
 		struct pq_entry top = merged_iter_pqueue_top(pq);
-		struct pq_entry e = merged_iter_pqueue_remove(&pq);
+		struct pq_entry e;
+
+		check(!merged_iter_pqueue_remove(&pq, &e));
 		merged_iter_pqueue_check(&pq);
 
 		check(pq_entry_equal(&top, &e));
@@ -129,7 +135,9 @@ static void t_merged_iter_pqueue_top(void)
 
 	for (i = N - 1; i > 0; i--) {
 		struct pq_entry top = merged_iter_pqueue_top(pq);
-		struct pq_entry e = merged_iter_pqueue_remove(&pq);
+		struct pq_entry e;
+
+		check(!merged_iter_pqueue_remove(&pq, &e));
 
 		merged_iter_pqueue_check(&pq);
 		check(pq_entry_equal(&top, &e));
diff --git a/t/unit-tests/t-reftable-record.c b/t/unit-tests/t-reftable-record.c
index 6540bd20e37..59549663736 100644
--- a/t/unit-tests/t-reftable-record.c
+++ b/t/unit-tests/t-reftable-record.c
@@ -100,16 +100,20 @@ static void t_reftable_ref_record_comparison(void)
 			.u.ref.value.symref = (char *) "refs/heads/master",
 		},
 	};
+	int cmp;
 
 	check(!reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check(!cmp);
 
 	check(!reftable_record_equal(&in[1], &in[2], REFTABLE_HASH_SIZE_SHA1));
-	check_int(reftable_record_cmp(&in[1], &in[2]), >, 0);
+	check(!reftable_record_cmp(&in[1], &in[2], &cmp));
+	check_int(cmp, >, 0);
 
 	in[1].u.ref.value_type = in[0].u.ref.value_type;
 	check(reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check(!cmp);
 }
 
 static void t_reftable_ref_record_compare_name(void)
@@ -209,17 +213,20 @@ static void t_reftable_log_record_comparison(void)
 			.u.log.update_index = 22,
 		},
 	};
+	int cmp;
 
 	check(!reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
 	check(!reftable_record_equal(&in[1], &in[2], REFTABLE_HASH_SIZE_SHA1));
-	check_int(reftable_record_cmp(&in[1], &in[2]), >, 0);
+	check(!reftable_record_cmp(&in[1], &in[2], &cmp));
+	check_int(cmp, >, 0);
 	/* comparison should be reversed for equal keys, because
 	 * comparison is now performed on the basis of update indices */
-	check_int(reftable_record_cmp(&in[0], &in[1]), <, 0);
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check_int(cmp, <, 0);
 
 	in[1].u.log.update_index = in[0].u.log.update_index;
 	check(reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
 }
 
 static void t_reftable_log_record_compare_key(void)
@@ -396,16 +403,20 @@ static void t_reftable_obj_record_comparison(void)
 			.u.obj.hash_prefix_len = 5,
 		},
 	};
+	int cmp;
 
 	check(!reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check(!cmp);
 
 	check(!reftable_record_equal(&in[1], &in[2], REFTABLE_HASH_SIZE_SHA1));
-	check_int(reftable_record_cmp(&in[1], &in[2]), >, 0);
+	check(!reftable_record_cmp(&in[1], &in[2], &cmp));
+	check_int(cmp, >, 0);
 
 	in[1].u.obj.offset_len = in[0].u.obj.offset_len;
 	check(reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check(!cmp);
 }
 
 static void t_reftable_obj_record_roundtrip(void)
@@ -486,19 +497,24 @@ static void t_reftable_index_record_comparison(void)
 			.u.idx.last_key = REFTABLE_BUF_INIT,
 		},
 	};
+	int cmp;
+
 	check(!reftable_buf_addstr(&in[0].u.idx.last_key, "refs/heads/master"));
 	check(!reftable_buf_addstr(&in[1].u.idx.last_key, "refs/heads/master"));
 	check(!reftable_buf_addstr(&in[2].u.idx.last_key, "refs/heads/branch"));
 
 	check(!reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check(!cmp);
 
 	check(!reftable_record_equal(&in[1], &in[2], REFTABLE_HASH_SIZE_SHA1));
-	check_int(reftable_record_cmp(&in[1], &in[2]), >, 0);
+	check(!reftable_record_cmp(&in[1], &in[2], &cmp));
+	check_int(cmp, >, 0);
 
 	in[1].u.idx.offset = in[0].u.idx.offset;
 	check(reftable_record_equal(&in[0], &in[1], REFTABLE_HASH_SIZE_SHA1));
-	check(!reftable_record_cmp(&in[0], &in[1]));
+	check(!reftable_record_cmp(&in[0], &in[1], &cmp));
+	check(!cmp);
 
 	for (size_t i = 0; i < ARRAY_SIZE(in); i++)
 		reftable_record_release(&in[i]);
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 08/18] reftable/basics: stop using `st_mult()` in array allocators

We're using `st_mult()` as part of our macro helpers that allocate arrays. This is bad due two two reasons:

  - `st_mult()` causes us to die in case the multiplication overflows.
  - `st_mult()` ties us to the Git codebase.

Refactor the code to instead detect overflows manually and return an error in such cases.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/basics.h | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)
Show changes to reftable/basics.h +32 −4
diff --git a/reftable/basics.h b/reftable/basics.h
index a2a010a0e19..646f8d67f2d 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -117,18 +117,46 @@ void reftable_free(void *p);
 void *reftable_calloc(size_t nelem, size_t elsize);
 char *reftable_strdup(const char *str);
 
-#define REFTABLE_ALLOC_ARRAY(x, alloc) (x) = reftable_malloc(st_mult(sizeof(*(x)), (alloc)))
+static inline int reftable_alloc_size(size_t nelem, size_t elsize, size_t *out)
+{
+	if (nelem && elsize > SIZE_MAX / nelem)
+		return -1;
+	*out = nelem * elsize;
+	return 0;
+}
+
+#define REFTABLE_ALLOC_ARRAY(x, alloc) do { \
+		size_t alloc_size; \
+		if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
+			errno = ENOMEM; \
+			(x) = NULL; \
+		} else { \
+			(x) = reftable_malloc(alloc_size); \
+		} \
+	} while (0)
 #define REFTABLE_CALLOC_ARRAY(x, alloc) (x) = reftable_calloc((alloc), sizeof(*(x)))
-#define REFTABLE_REALLOC_ARRAY(x, alloc) (x) = reftable_realloc((x), st_mult(sizeof(*(x)), (alloc)))
+#define REFTABLE_REALLOC_ARRAY(x, alloc) do { \
+		size_t alloc_size; \
+		if (reftable_alloc_size(sizeof(*(x)), (alloc), &alloc_size) < 0) { \
+			errno = ENOMEM; \
+			(x) = NULL; \
+		} else { \
+			(x) = reftable_realloc((x), alloc_size); \
+		} \
+	} while (0)
 
 static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
 					size_t *allocp)
 {
 	void *new_p;
-	size_t alloc = *allocp * 2 + 1;
+	size_t alloc = *allocp * 2 + 1, alloc_bytes;
 	if (alloc < nelem)
 		alloc = nelem;
-	new_p = reftable_realloc(p, st_mult(elsize, alloc));
+	if (reftable_alloc_size(elsize, alloc, &alloc_bytes) < 0) {
+		errno = ENOMEM;
+		return p;
+	}
+	new_p = reftable_realloc(p, alloc_bytes);
 	if (!new_p)
 		return p;
 	*allocp = alloc;
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 07/18] reftable: stop using `BUG()` in trivial cases

Stop using `BUG()` in the remaining trivial cases that we still have in the reftable library. Instead of aborting the program, we'll now bubble up a `REFTABLE_API_ERROR` to indicate misuse of the calling conventions.

Note that in both `reftable_reader_{inc,dec}ref()` we simply stop calling `BUG()` altogether. The only situation where the counter should be zero is when the structure has already been free'd anyway, so we would run into undefined behaviour regardless of whether we try to abort the program or not.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/iter.c   | 3 +--
 reftable/reader.c | 4 ----
 reftable/writer.c | 5 ++---
 3 files changed, 3 insertions(+), 9 deletions(-)
Show changes to 3 files +3 −9

reftable/iter.c, reftable/reader.c, reftable/writer.c

diff --git a/reftable/iter.c b/reftable/iter.c
index 86e801ca9fb..b2ffb09c16b 100644
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -146,8 +146,7 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
 static int indexed_table_ref_iter_seek(void *p UNUSED,
 				       struct reftable_record *want UNUSED)
 {
-	BUG("seeking indexed table is not supported");
-	return -1;
+	return REFTABLE_API_ERROR;
 }
 
 static int indexed_table_ref_iter_next(void *p, struct reftable_record *rec)
diff --git a/reftable/reader.c b/reftable/reader.c
index de6e6dd9322..36a5633eded 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -677,8 +677,6 @@ int reftable_reader_new(struct reftable_reader **out,
 
 void reftable_reader_incref(struct reftable_reader *r)
 {
-	if (!r->refcount)
-		BUG("cannot increment ref counter of dead reader");
 	r->refcount++;
 }
 
@@ -686,8 +684,6 @@ void reftable_reader_decref(struct reftable_reader *r)
 {
 	if (!r)
 		return;
-	if (!r->refcount)
-		BUG("cannot decrement ref counter of dead reader");
 	if (--r->refcount)
 		return;
 	block_source_close(&r->source);
diff --git a/reftable/writer.c b/reftable/writer.c
index f3ab1035d61..239573ade24 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -158,7 +158,7 @@ int reftable_writer_new(struct reftable_writer **out,
 		opts = *_opts;
 	options_set_defaults(&opts);
 	if (opts.block_size >= (1 << 24))
-		BUG("configured block size exceeds 16MB");
+		return REFTABLE_API_ERROR;
 
 	reftable_buf_init(&wp->block_writer_data.last_key);
 	reftable_buf_init(&wp->last_key);
@@ -302,8 +302,7 @@ static int writer_add_record(struct reftable_writer *w,
 	}
 
 	if (block_writer_type(w->block_writer) != reftable_record_type(rec))
-		BUG("record of type %d added to writer of type %d",
-		    reftable_record_type(rec), block_writer_type(w->block_writer));
+		return REFTABLE_API_ERROR;
 
 	/*
 	 * Try to add the record to the writer. If this succeeds then we're
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 10/18] reftable/reader: stop using `ARRAY_SIZE()` macro

We have a single user of the `ARRAY_SIZE()` macro in the reftable reader. Drop its use to reduce our dependence on the Git codebase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/reader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to reftable/reader.c +1 −1
diff --git a/reftable/reader.c b/reftable/reader.c
index bf07a0a586f..c3a36746652 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -849,7 +849,7 @@ int reftable_reader_print_blocks(const char *tablename)
 	printf("header:\n");
 	printf("  block_size: %d\n", r->block_size);
 
-	for (i = 0; i < ARRAY_SIZE(sections); i++) {
+	for (i = 0; i < sizeof(sections) / sizeof(*sections); i++) {
 		err = table_iter_seek_start(&ti, sections[i].type, 0);
 		if (err < 0)
 			goto done;
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 09/18] reftable/basics: provide wrappers for big endian conversion

We're using a mixture of big endian conversion functions provided by both the reftable library, but also by the Git codebase. Refactor the code so that we exclusively use reftable-provided wrappers in order to untangle us from the Git codebase.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/basics.c                | 19 ----------
 reftable/basics.h                | 76 ++++++++++++++++++++++++++++++++++++++--
 reftable/block.c                 | 12 +++----
 reftable/reader.c                | 22 ++++++------
 reftable/record.c                |  8 ++---
 reftable/writer.c                | 20 +++++------
 t/unit-tests/t-reftable-basics.c | 28 ++++++++++++---
 7 files changed, 127 insertions(+), 58 deletions(-)
Show changes to 7 files +127 −58

reftable/basics.c, reftable/basics.h, reftable/block.c, reftable/reader.c, reftable/record.c, reftable/writer.c, t/unit-tests/t-reftable-basics.c

diff --git a/reftable/basics.c b/reftable/basics.c
index 3b5ea27bbdc..8c4a4433e41 100644
--- a/reftable/basics.c
+++ b/reftable/basics.c
@@ -147,25 +147,6 @@ char *reftable_buf_detach(struct reftable_buf *buf)
 	return result;
 }
 
-void put_be24(uint8_t *out, uint32_t i)
-{
-	out[0] = (uint8_t)((i >> 16) & 0xff);
-	out[1] = (uint8_t)((i >> 8) & 0xff);
-	out[2] = (uint8_t)(i & 0xff);
-}
-
-uint32_t get_be24(uint8_t *in)
-{
-	return (uint32_t)(in[0]) << 16 | (uint32_t)(in[1]) << 8 |
-	       (uint32_t)(in[2]);
-}
-
-void put_be16(uint8_t *out, uint16_t i)
-{
-	out[0] = (uint8_t)((i >> 8) & 0xff);
-	out[1] = (uint8_t)(i & 0xff);
-}
-
 size_t binsearch(size_t sz, int (*f)(size_t k, void *args), void *args)
 {
 	size_t lo = 0;
diff --git a/reftable/basics.h b/reftable/basics.h
index 646f8d67f2d..c1ddbaec3ff 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -76,9 +76,79 @@ char *reftable_buf_detach(struct reftable_buf *buf);
 
 /* Bigendian en/decoding of integers */
 
-void put_be24(uint8_t *out, uint32_t i);
-uint32_t get_be24(uint8_t *in);
-void put_be16(uint8_t *out, uint16_t i);
+static inline void reftable_put_be16(void *out, uint16_t i)
+{
+	unsigned char *p = out;
+	p[0] = (uint8_t)((i >> 8) & 0xff);
+	p[1] = (uint8_t)((i >> 0) & 0xff);
+}
+
+static inline void reftable_put_be24(void *out, uint32_t i)
+{
+	unsigned char *p = out;
+	p[0] = (uint8_t)((i >> 16) & 0xff);
+	p[1] = (uint8_t)((i >>  8) & 0xff);
+	p[2] = (uint8_t)((i >>  0) & 0xff);
+}
+
+static inline void reftable_put_be32(void *out, uint32_t i)
+{
+	unsigned char *p = out;
+	p[0] = (uint8_t)((i >> 24) & 0xff);
+	p[1] = (uint8_t)((i >> 16) & 0xff);
+	p[2] = (uint8_t)((i >>  8) & 0xff);
+	p[3] = (uint8_t)((i >>  0) & 0xff);
+}
+
+static inline void reftable_put_be64(void *out, uint64_t i)
+{
+	unsigned char *p = out;
+	p[0] = (uint8_t)((i >> 56) & 0xff);
+	p[1] = (uint8_t)((i >> 48) & 0xff);
+	p[2] = (uint8_t)((i >> 40) & 0xff);
+	p[3] = (uint8_t)((i >> 32) & 0xff);
+	p[4] = (uint8_t)((i >> 24) & 0xff);
+	p[5] = (uint8_t)((i >> 16) & 0xff);
+	p[6] = (uint8_t)((i >>  8) & 0xff);
+	p[7] = (uint8_t)((i >>  0) & 0xff);
+}
+
+static inline uint16_t reftable_get_be16(const void *in)
+{
+	const unsigned char *p = in;
+	return (uint16_t)(p[0]) << 8 |
+	       (uint16_t)(p[1]) << 0;
+}
+
+static inline uint32_t reftable_get_be24(const void *in)
+{
+	const unsigned char *p = in;
+	return (uint32_t)(p[0]) << 16 |
+	       (uint32_t)(p[1]) << 8 |
+	       (uint32_t)(p[2]) << 0;
+}
+
+static inline uint32_t reftable_get_be32(const void *in)
+{
+	const unsigned char *p = in;
+	return (uint32_t)(p[0]) << 24 |
+	       (uint32_t)(p[1]) << 16 |
+	       (uint32_t)(p[2]) <<  8|
+	       (uint32_t)(p[3]) <<  0;
+}
+
+static inline uint64_t reftable_get_be64(const void *in)
+{
+	const unsigned char *p = in;
+	return (uint64_t)(p[0]) << 56 |
+	       (uint64_t)(p[1]) << 48 |
+	       (uint64_t)(p[2]) << 40 |
+	       (uint64_t)(p[3]) << 32 |
+	       (uint64_t)(p[4]) << 24 |
+	       (uint64_t)(p[5]) << 16 |
+	       (uint64_t)(p[6]) <<  8 |
+	       (uint64_t)(p[7]) <<  0;
+}
 
 /*
  * find smallest index i in [0, sz) at which `f(i) > 0`, assuming that f is
diff --git a/reftable/block.c b/reftable/block.c
index 999876826d1..53b5e044690 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -147,13 +147,13 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
 int block_writer_finish(struct block_writer *w)
 {
 	for (uint32_t i = 0; i < w->restart_len; i++) {
-		put_be24(w->block + w->next, w->restarts[i]);
+		reftable_put_be24(w->block + w->next, w->restarts[i]);
 		w->next += 3;
 	}
 
-	put_be16(w->block + w->next, w->restart_len);
+	reftable_put_be16(w->block + w->next, w->restart_len);
 	w->next += 2;
-	put_be24(w->block + 1 + w->header_off, w->next);
+	reftable_put_be24(w->block + 1 + w->header_off, w->next);
 
 	/*
 	 * Log records are stored zlib-compressed. Note that the compression
@@ -215,7 +215,7 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
 {
 	uint32_t full_block_size = table_block_size;
 	uint8_t typ = block->data[header_off];
-	uint32_t sz = get_be24(block->data + header_off + 1);
+	uint32_t sz = reftable_get_be24(block->data + header_off + 1);
 	int err = 0;
 	uint16_t restart_count = 0;
 	uint32_t restart_start = 0;
@@ -299,7 +299,7 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
 		full_block_size = sz;
 	}
 
-	restart_count = get_be16(block->data + sz - 2);
+	restart_count = reftable_get_be16(block->data + sz - 2);
 	restart_start = sz - 2 - 3 * restart_count;
 	restart_bytes = block->data + restart_start;
 
@@ -354,7 +354,7 @@ int block_reader_first_key(const struct block_reader *br, struct reftable_buf *k
 
 static uint32_t block_reader_restart_offset(const struct block_reader *br, size_t idx)
 {
-	return get_be24(br->restart_bytes + 3 * idx);
+	return reftable_get_be24(br->restart_bytes + 3 * idx);
 }
 
 void block_iter_seek_start(struct block_iter *it, const struct block_reader *br)
diff --git a/reftable/reader.c b/reftable/reader.c
index 36a5633eded..bf07a0a586f 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -101,18 +101,18 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,
 	}
 
 	f++;
-	r->block_size = get_be24(f);
+	r->block_size = reftable_get_be24(f);
 
 	f += 3;
-	r->min_update_index = get_be64(f);
+	r->min_update_index = reftable_get_be64(f);
 	f += 8;
-	r->max_update_index = get_be64(f);
+	r->max_update_index = reftable_get_be64(f);
 	f += 8;
 
 	if (r->version == 1) {
 		r->hash_id = REFTABLE_HASH_SHA1;
 	} else {
-		switch (get_be32(f)) {
+		switch (reftable_get_be32(f)) {
 		case REFTABLE_FORMAT_ID_SHA1:
 			r->hash_id = REFTABLE_HASH_SHA1;
 			break;
@@ -127,24 +127,24 @@ static int parse_footer(struct reftable_reader *r, uint8_t *footer,
 		f += 4;
 	}
 
-	r->ref_offsets.index_offset = get_be64(f);
+	r->ref_offsets.index_offset = reftable_get_be64(f);
 	f += 8;
 
-	r->obj_offsets.offset = get_be64(f);
+	r->obj_offsets.offset = reftable_get_be64(f);
 	f += 8;
 
 	r->object_id_len = r->obj_offsets.offset & ((1 << 5) - 1);
 	r->obj_offsets.offset >>= 5;
 
-	r->obj_offsets.index_offset = get_be64(f);
+	r->obj_offsets.index_offset = reftable_get_be64(f);
 	f += 8;
-	r->log_offsets.offset = get_be64(f);
+	r->log_offsets.offset = reftable_get_be64(f);
 	f += 8;
-	r->log_offsets.index_offset = get_be64(f);
+	r->log_offsets.index_offset = reftable_get_be64(f);
 	f += 8;
 
 	computed_crc = crc32(0, footer, f - footer);
-	file_crc = get_be32(f);
+	file_crc = reftable_get_be32(f);
 	f += 4;
 	if (computed_crc != file_crc) {
 		err = REFTABLE_FORMAT_ERROR;
@@ -214,7 +214,7 @@ static int32_t extract_block_size(uint8_t *data, uint8_t *typ, uint64_t off,
 
 	*typ = data[0];
 	if (reftable_is_block_type(*typ)) {
-		result = get_be24(data + 1);
+		result = reftable_get_be24(data + 1);
 	}
 	return result;
 }
diff --git a/reftable/record.c b/reftable/record.c
index b39d99fcc75..3552bafa994 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -689,7 +689,7 @@ static int reftable_log_record_key(const void *r, struct reftable_buf *dest)
 		return err;
 
 	ts = (~ts) - rec->update_index;
-	put_be64(&i64[0], ts);
+	reftable_put_be64(&i64[0], ts);
 
 	err = reftable_buf_add(dest, i64, sizeof(i64));
 	if (err < 0)
@@ -814,7 +814,7 @@ static int reftable_log_record_encode(const void *rec, struct string_view s,
 	if (s.len < 2)
 		return -1;
 
-	put_be16(s.buf, r->value.update.tz_offset);
+	reftable_put_be16(s.buf, r->value.update.tz_offset);
 	string_view_consume(&s, 2);
 
 	n = encode_string(
@@ -846,7 +846,7 @@ static int reftable_log_record_decode(void *rec, struct reftable_buf key,
 	}
 
 	memcpy(r->refname, key.buf, key.len - 8);
-	ts = get_be64(key.buf + key.len - 8);
+	ts = reftable_get_be64((unsigned char *)key.buf + key.len - 8);
 
 	r->update_index = (~max) - ts;
 
@@ -937,7 +937,7 @@ static int reftable_log_record_decode(void *rec, struct reftable_buf key,
 		goto done;
 	}
 
-	r->value.update.tz_offset = get_be16(in.buf);
+	r->value.update.tz_offset = reftable_get_be16(in.buf);
 	string_view_consume(&in, 2);
 
 	n = decode_string(scratch, in);
diff --git a/reftable/writer.c b/reftable/writer.c
index 239573ade24..913b971b591 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -99,9 +99,9 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)
 
 	dest[4] = writer_version(w);
 
-	put_be24(dest + 5, w->opts.block_size);
-	put_be64(dest + 8, w->min_update_index);
-	put_be64(dest + 16, w->max_update_index);
+	reftable_put_be24(dest + 5, w->opts.block_size);
+	reftable_put_be64(dest + 8, w->min_update_index);
+	reftable_put_be64(dest + 16, w->max_update_index);
 	if (writer_version(w) == 2) {
 		uint32_t hash_id;
 
@@ -116,7 +116,7 @@ static int writer_write_header(struct reftable_writer *w, uint8_t *dest)
 			return -1;
 		}
 
-		put_be32(dest + 24, hash_id);
+		reftable_put_be32(dest + 24, hash_id);
 	}
 
 	return header_size(writer_version(w));
@@ -730,19 +730,19 @@ int reftable_writer_close(struct reftable_writer *w)
 	}
 
 	p += writer_write_header(w, footer);
-	put_be64(p, w->stats.ref_stats.index_offset);
+	reftable_put_be64(p, w->stats.ref_stats.index_offset);
 	p += 8;
-	put_be64(p, (w->stats.obj_stats.offset) << 5 | w->stats.object_id_len);
+	reftable_put_be64(p, (w->stats.obj_stats.offset) << 5 | w->stats.object_id_len);
 	p += 8;
-	put_be64(p, w->stats.obj_stats.index_offset);
+	reftable_put_be64(p, w->stats.obj_stats.index_offset);
 	p += 8;
 
-	put_be64(p, w->stats.log_stats.offset);
+	reftable_put_be64(p, w->stats.log_stats.offset);
 	p += 8;
-	put_be64(p, w->stats.log_stats.index_offset);
+	reftable_put_be64(p, w->stats.log_stats.index_offset);
 	p += 8;
 
-	put_be32(p, crc32(0, footer, p - footer));
+	reftable_put_be32(p, crc32(0, footer, p - footer));
 	p += 4;
 
 	err = w->flush(w->write_arg);
diff --git a/t/unit-tests/t-reftable-basics.c b/t/unit-tests/t-reftable-basics.c
index 9ba7eb05ada..c9e751e49e9 100644
--- a/t/unit-tests/t-reftable-basics.c
+++ b/t/unit-tests/t-reftable-basics.c
@@ -128,12 +128,30 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 		reftable_buf_release(&b);
 	}
 
-	if_test ("put_be24 and get_be24 work") {
+	if_test ("reftable_put_be64 and reftable_get_be64 work") {
+		uint64_t in = 0x1122334455667788;
+		uint8_t dest[8];
+		uint64_t out;
+		reftable_put_be64(dest, in);
+		out = reftable_get_be64(dest);
+		check_int(in, ==, out);
+	}
+
+	if_test ("reftable_put_be32 and reftable_get_be32 work") {
+		uint32_t in = 0x11223344;
+		uint8_t dest[4];
+		uint32_t out;
+		reftable_put_be32(dest, in);
+		out = reftable_get_be32(dest);
+		check_int(in, ==, out);
+	}
+
+	if_test ("reftable_put_be24 and reftable_get_be24 work") {
 		uint32_t in = 0x112233;
 		uint8_t dest[3];
 		uint32_t out;
-		put_be24(dest, in);
-		out = get_be24(dest);
+		reftable_put_be24(dest, in);
+		out = reftable_get_be24(dest);
 		check_int(in, ==, out);
 	}
 
@@ -141,8 +159,8 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
 		uint32_t in = 0xfef1;
 		uint8_t dest[3];
 		uint32_t out;
-		put_be16(dest, in);
-		out = get_be16(dest);
+		reftable_put_be16(dest, in);
+		out = reftable_get_be16(dest);
 		check_int(in, ==, out);
 	}
 
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 11/18] reftable/system: introduce `reftable_rand()`

Introduce a new system-level `reftable_rand()` function that generates a single unsigned integer for us. The implementation of this function is to be provided by the calling codebase, which allows us to more easily hook into pre-seeded random number generators.

Adapt the two callsites where we generated random data.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c  | 4 ++--
 reftable/system.c | 5 +++++
 reftable/system.h | 6 ++++++
 3 files changed, 13 insertions(+), 2 deletions(-)
Show changes to 3 files +13 −2

reftable/stack.c, reftable/system.c, reftable/system.h

diff --git a/reftable/stack.c b/reftable/stack.c
index c33f0c3333a..08893fd454f 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -523,7 +523,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
 		close(fd);
 		fd = -1;
 
-		delay = delay + (delay * git_rand(CSPRNG_BYTES_INSECURE)) / UINT32_MAX + 1;
+		delay = delay + (delay * reftable_rand()) / UINT32_MAX + 1;
 		sleep_millisec(delay);
 	}
 
@@ -688,7 +688,7 @@ int reftable_stack_add(struct reftable_stack *st,
 static int format_name(struct reftable_buf *dest, uint64_t min, uint64_t max)
 {
 	char buf[100];
-	uint32_t rnd = git_rand(CSPRNG_BYTES_INSECURE);
+	uint32_t rnd = reftable_rand();
 	snprintf(buf, sizeof(buf), "0x%012" PRIx64 "-0x%012" PRIx64 "-%08x",
 		 min, max, rnd);
 	reftable_buf_reset(dest);
diff --git a/reftable/system.c b/reftable/system.c
index adf8e4d30b8..e25ccc0da3c 100644
--- a/reftable/system.c
+++ b/reftable/system.c
@@ -4,6 +4,11 @@
 #include "../lockfile.h"
 #include "../tempfile.h"
 
+uint32_t reftable_rand(void)
+{
+	return git_rand(CSPRNG_BYTES_INSECURE);
+}
+
 int tmpfile_from_pattern(struct reftable_tmpfile *out, const char *pattern)
 {
 	struct tempfile *tempfile;
diff --git a/reftable/system.h b/reftable/system.h
index d02eacea8f0..bb6a7e6285b 100644
--- a/reftable/system.h
+++ b/reftable/system.h
@@ -14,6 +14,12 @@ license that can be found in the LICENSE file or at
 #include "git-compat-util.h"
 #include "compat/zlib-compat.h"
 
+/*
+ * Return a random 32 bit integer. This function is expected to return
+ * pre-seeded data.
+ */
+uint32_t reftable_rand(void);
+
 /*
  * An implementation-specific temporary file. By making this specific to the
  * implementation it becomes possible to tie temporary files into any kind of
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 12/18] reftable/stack: stop using `sleep_millisec()`

Refactor our use of `sleep_millisec()` by open-coding it with poll(3p), which is the current implementation of this function. Ideally, we'd use a more direct way to sleep, but there is no equivalent to sleep(3p) that would accept milliseconds as input.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/stack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to reftable/stack.c +1 −1
diff --git a/reftable/stack.c b/reftable/stack.c
index 08893fd454f..6dac015b473 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -524,7 +524,7 @@ static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
 		fd = -1;
 
 		delay = delay + (delay * reftable_rand()) / UINT32_MAX + 1;
-		sleep_millisec(delay);
+		poll(NULL, 0, delay);
 	}
 
 out:
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 13/18] reftable/basics: stop using `SWAP()` macro

Stop using `SWAP()` macro in favor of an open-coded variant of it. Note that this also requires us to open-code the build assert that `SWAP()` itself uses to verify that the size of both variables matches.

This is done to reduce our dependency on the Git codebase.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/basics.h |  9 +++++++++
 reftable/merged.c |  2 +-
 reftable/pq.c     |  4 ++--
 reftable/record.c | 16 ++++++++--------
 4 files changed, 20 insertions(+), 11 deletions(-)
Show changes to 4 files +20 −11

reftable/basics.h, reftable/merged.c, reftable/pq.c, reftable/record.c

diff --git a/reftable/basics.h b/reftable/basics.h
index c1ddbaec3ff..59000798f0b 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -266,6 +266,15 @@ static inline void *reftable_alloc_grow(void *p, size_t nelem, size_t elsize,
 # define strdup(str) REFTABLE_BANNED(strdup)
 #endif
 
+#define REFTABLE_SWAP(a, b) do {								\
+	void *_swap_a_ptr = &(a);								\
+	void *_swap_b_ptr = &(b);								\
+	unsigned char _swap_buffer[sizeof(a) - 2 * sizeof(a) * (sizeof(a) != sizeof(b))];	\
+	memcpy(_swap_buffer, _swap_a_ptr, sizeof(a));						\
+	memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a));						\
+	memcpy(_swap_b_ptr, _swap_buffer, sizeof(a));						\
+} while (0)
+
 /* Find the longest shared prefix size of `a` and `b` */
 size_t common_prefix_size(struct reftable_buf *a, struct reftable_buf *b);
 
diff --git a/reftable/merged.c b/reftable/merged.c
index 563864068c1..4ff1553772a 100644
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -155,7 +155,7 @@ static int merged_iter_next_entry(struct merged_iter *mi,
 	}
 
 	mi->advance_index = entry.index;
-	SWAP(*rec, *entry.rec);
+	REFTABLE_SWAP(*rec, *entry.rec);
 	return 0;
 }
 
diff --git a/reftable/pq.c b/reftable/pq.c
index ef8035cfd9b..82394a972db 100644
--- a/reftable/pq.c
+++ b/reftable/pq.c
@@ -57,7 +57,7 @@ int merged_iter_pqueue_remove(struct merged_iter_pqueue *pq, struct pq_entry *ou
 
 		if (min == i)
 			break;
-		SWAP(pq->heap[i], pq->heap[min]);
+		REFTABLE_SWAP(pq->heap[i], pq->heap[min]);
 		i = min;
 	}
 
@@ -81,7 +81,7 @@ int merged_iter_pqueue_add(struct merged_iter_pqueue *pq, const struct pq_entry
 		size_t j = (i - 1) / 2;
 		if (pq_less(&pq->heap[j], &pq->heap[i]))
 			break;
-		SWAP(pq->heap[j], pq->heap[i]);
+		REFTABLE_SWAP(pq->heap[j], pq->heap[i]);
 		i = j;
 	}
 
diff --git a/reftable/record.c b/reftable/record.c
index 3552bafa994..9a1edf39a02 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -237,11 +237,11 @@ static int reftable_ref_record_copy_from(void *rec, const void *src_rec,
 	size_t refname_cap = 0;
 	int err;
 
-	SWAP(refname, ref->refname);
-	SWAP(refname_cap, ref->refname_cap);
+	REFTABLE_SWAP(refname, ref->refname);
+	REFTABLE_SWAP(refname_cap, ref->refname_cap);
 	reftable_ref_record_release(ref);
-	SWAP(ref->refname, refname);
-	SWAP(ref->refname_cap, refname_cap);
+	REFTABLE_SWAP(ref->refname, refname);
+	REFTABLE_SWAP(ref->refname_cap, refname_cap);
 
 	if (src->refname) {
 		size_t refname_len = strlen(src->refname);
@@ -376,11 +376,11 @@ static int reftable_ref_record_decode(void *rec, struct reftable_buf key,
 		return n;
 	string_view_consume(&in, n);
 
-	SWAP(refname, r->refname);
-	SWAP(refname_cap, r->refname_cap);
+	REFTABLE_SWAP(refname, r->refname);
+	REFTABLE_SWAP(refname_cap, r->refname_cap);
 	reftable_ref_record_release(r);
-	SWAP(r->refname, refname);
-	SWAP(r->refname_cap, refname_cap);
+	REFTABLE_SWAP(r->refname, refname);
+	REFTABLE_SWAP(r->refname_cap, refname_cap);
 
 	REFTABLE_ALLOC_GROW_OR_NULL(r->refname, key.len + 1, r->refname_cap);
 	if (!r->refname) {
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 14/18] reftable/basics: introduce `REFTABLE_UNUSED` annotation

Introduce the `REFTABLE_UNUSED` annotation and replace all existing users of `UNUSED` in the reftable library to use the new macro instead.

Note that we unconditionally define `MAYBE_UNUSED` in the exact same way, so doing so unconditionally for `REFTABLE_UNUSED` should be fine, too.

Suggested-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/basics.h      |  2 ++
 reftable/blocksource.c |  6 +++---
 reftable/iter.c        | 10 +++++-----
 reftable/record.c      | 26 +++++++++++++-------------
 reftable/writer.c      |  2 +-
 5 files changed, 24 insertions(+), 22 deletions(-)
Show changes to 5 files +24 −22

reftable/basics.h, reftable/blocksource.c, reftable/iter.c, reftable/record.c, reftable/writer.c

diff --git a/reftable/basics.h b/reftable/basics.h
index 59000798f0b..fd59cbb7720 100644
--- a/reftable/basics.h
+++ b/reftable/basics.h
@@ -16,6 +16,8 @@ license that can be found in the LICENSE file or at
 #include "system.h"
 #include "reftable-basics.h"
 
+#define REFTABLE_UNUSED __attribute__((__unused__))
+
 struct reftable_buf {
 	size_t alloc;
 	size_t len;
diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 02972c46f4b..78c1be23373 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -13,14 +13,14 @@ license that can be found in the LICENSE file or at
 #include "reftable-blocksource.h"
 #include "reftable-error.h"
 
-static void reftable_buf_return_block(void *b UNUSED, struct reftable_block *dest)
+static void reftable_buf_return_block(void *b REFTABLE_UNUSED, struct reftable_block *dest)
 {
 	if (dest->len)
 		memset(dest->data, 0xff, dest->len);
 	reftable_free(dest->data);
 }
 
-static void reftable_buf_close(void *b UNUSED)
+static void reftable_buf_close(void *b REFTABLE_UNUSED)
 {
 }
 
@@ -67,7 +67,7 @@ static uint64_t file_size(void *b)
 	return ((struct file_block_source *)b)->size;
 }
 
-static void file_return_block(void *b UNUSED, struct reftable_block *dest UNUSED)
+static void file_return_block(void *b REFTABLE_UNUSED, struct reftable_block *dest REFTABLE_UNUSED)
 {
 }
 
diff --git a/reftable/iter.c b/reftable/iter.c
index b2ffb09c16b..f520382e700 100644
--- a/reftable/iter.c
+++ b/reftable/iter.c
@@ -25,17 +25,17 @@ int iterator_next(struct reftable_iterator *it, struct reftable_record *rec)
 	return it->ops->next(it->iter_arg, rec);
 }
 
-static int empty_iterator_seek(void *arg UNUSED, struct reftable_record *want UNUSED)
+static int empty_iterator_seek(void *arg REFTABLE_UNUSED, struct reftable_record *want REFTABLE_UNUSED)
 {
 	return 0;
 }
 
-static int empty_iterator_next(void *arg UNUSED, struct reftable_record *rec UNUSED)
+static int empty_iterator_next(void *arg REFTABLE_UNUSED, struct reftable_record *rec REFTABLE_UNUSED)
 {
 	return 1;
 }
 
-static void empty_iterator_close(void *arg UNUSED)
+static void empty_iterator_close(void *arg REFTABLE_UNUSED)
 {
 }
 
@@ -143,8 +143,8 @@ static int indexed_table_ref_iter_next_block(struct indexed_table_ref_iter *it)
 	return 0;
 }
 
-static int indexed_table_ref_iter_seek(void *p UNUSED,
-				       struct reftable_record *want UNUSED)
+static int indexed_table_ref_iter_seek(void *p REFTABLE_UNUSED,
+				       struct reftable_record *want REFTABLE_UNUSED)
 {
 	return REFTABLE_API_ERROR;
 }
diff --git a/reftable/record.c b/reftable/record.c
index 9a1edf39a02..142853d5070 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -490,7 +490,7 @@ static void reftable_obj_record_release(void *rec)
 }
 
 static int reftable_obj_record_copy_from(void *rec, const void *src_rec,
-					 uint32_t hash_size UNUSED)
+					 uint32_t hash_size REFTABLE_UNUSED)
 {
 	struct reftable_obj_record *obj = rec;
 	const struct reftable_obj_record *src = src_rec;
@@ -528,7 +528,7 @@ static uint8_t reftable_obj_record_val_type(const void *rec)
 }
 
 static int reftable_obj_record_encode(const void *rec, struct string_view s,
-				      uint32_t hash_size UNUSED)
+				      uint32_t hash_size REFTABLE_UNUSED)
 {
 	const struct reftable_obj_record *r = rec;
 	struct string_view start = s;
@@ -563,8 +563,8 @@ static int reftable_obj_record_encode(const void *rec, struct string_view s,
 
 static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
 				      uint8_t val_type, struct string_view in,
-				      uint32_t hash_size UNUSED,
-				      struct reftable_buf *scratch UNUSED)
+				      uint32_t hash_size REFTABLE_UNUSED,
+				      struct reftable_buf *scratch REFTABLE_UNUSED)
 {
 	struct string_view start = in;
 	struct reftable_obj_record *r = rec;
@@ -618,13 +618,13 @@ static int reftable_obj_record_decode(void *rec, struct reftable_buf key,
 	return start.len - in.len;
 }
 
-static int not_a_deletion(const void *p UNUSED)
+static int not_a_deletion(const void *p REFTABLE_UNUSED)
 {
 	return 0;
 }
 
 static int reftable_obj_record_equal_void(const void *a, const void *b,
-					  uint32_t hash_size UNUSED)
+					  uint32_t hash_size REFTABLE_UNUSED)
 {
 	struct reftable_obj_record *ra = (struct reftable_obj_record *) a;
 	struct reftable_obj_record *rb = (struct reftable_obj_record *) b;
@@ -1054,7 +1054,7 @@ static int reftable_index_record_key(const void *r, struct reftable_buf *dest)
 }
 
 static int reftable_index_record_copy_from(void *rec, const void *src_rec,
-					   uint32_t hash_size UNUSED)
+					   uint32_t hash_size REFTABLE_UNUSED)
 {
 	struct reftable_index_record *dst = rec;
 	const struct reftable_index_record *src = src_rec;
@@ -1075,13 +1075,13 @@ static void reftable_index_record_release(void *rec)
 	reftable_buf_release(&idx->last_key);
 }
 
-static uint8_t reftable_index_record_val_type(const void *rec UNUSED)
+static uint8_t reftable_index_record_val_type(const void *rec REFTABLE_UNUSED)
 {
 	return 0;
 }
 
 static int reftable_index_record_encode(const void *rec, struct string_view out,
-					uint32_t hash_size UNUSED)
+					uint32_t hash_size REFTABLE_UNUSED)
 {
 	const struct reftable_index_record *r =
 		(const struct reftable_index_record *)rec;
@@ -1097,10 +1097,10 @@ static int reftable_index_record_encode(const void *rec, struct string_view out,
 }
 
 static int reftable_index_record_decode(void *rec, struct reftable_buf key,
-					uint8_t val_type UNUSED,
+					uint8_t val_type REFTABLE_UNUSED,
 					struct string_view in,
-					uint32_t hash_size UNUSED,
-					struct reftable_buf *scratch UNUSED)
+					uint32_t hash_size REFTABLE_UNUSED,
+					struct reftable_buf *scratch REFTABLE_UNUSED)
 {
 	struct string_view start = in;
 	struct reftable_index_record *r = rec;
@@ -1120,7 +1120,7 @@ static int reftable_index_record_decode(void *rec, struct reftable_buf key,
 }
 
 static int reftable_index_record_equal(const void *a, const void *b,
-				       uint32_t hash_size UNUSED)
+				       uint32_t hash_size REFTABLE_UNUSED)
 {
 	struct reftable_index_record *ia = (struct reftable_index_record *) a;
 	struct reftable_index_record *ib = (struct reftable_index_record *) b;
diff --git a/reftable/writer.c b/reftable/writer.c
index 913b971b591..ce55a1deb06 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -649,7 +649,7 @@ static void write_object_record(void *void_arg, void *key)
 done:;
 }
 
-static void object_record_free(void *void_arg UNUSED, void *key)
+static void object_record_free(void *void_arg REFTABLE_UNUSED, void *key)
 {
 	struct obj_index_tree_node *entry = key;
 
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 15/18] compat/mingw: split out POSIX-related bits

Split out POSIX-related bits from "compat/mingw.h" and "compat/msvc.h". This is in preparation for splitting up "git-compat-utils.h" into a header that provides POSIX-compatibility and a header that provides common wrappers used by the Git project.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 compat/mingw-posix.h | 431 +++++++++++++++++++++++++++++++++++++++++++++++++++
 compat/mingw.h       | 426 +-------------------------------------------------
 compat/msvc-posix.h  |  33 ++++
 compat/msvc.h        |  30 +---
 4 files changed, 467 insertions(+), 453 deletions(-)
Show changes to 4 files +467 −453

compat/mingw-posix.h, compat/mingw.h, compat/msvc-posix.h, compat/msvc.h

diff --git a/compat/mingw-posix.h b/compat/mingw-posix.h
new file mode 100644
index 00000000000..8dddfa818d6
--- /dev/null
+++ b/compat/mingw-posix.h
@@ -0,0 +1,431 @@
+#ifndef COMPAT_MINGW_POSIX_H
+#define COMPAT_MINGW_POSIX_H
+
+#ifdef __MINGW64_VERSION_MAJOR
+#include <stdint.h>
+#include <wchar.h>
+typedef _sigset_t sigset_t;
+#endif
+#include <winsock2.h>
+#include <ws2tcpip.h>
+
+/* MinGW-w64 reports to have flockfile, but it does not actually have it. */
+#ifdef __MINGW64_VERSION_MAJOR
+#undef _POSIX_THREAD_SAFE_FUNCTIONS
+#endif
+
+/*
+ * things that are not available in header files
+ */
+
+typedef int uid_t;
+typedef int socklen_t;
+#ifndef __MINGW64_VERSION_MAJOR
+typedef int pid_t;
+#define hstrerror strerror
+#endif
+
+#define S_IFLNK    0120000 /* Symbolic link */
+#define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
+#define S_ISSOCK(x) 0
+
+#ifndef S_IRWXG
+#define S_IRGRP 0
+#define S_IWGRP 0
+#define S_IXGRP 0
+#define S_IRWXG (S_IRGRP | S_IWGRP | S_IXGRP)
+#endif
+#ifndef S_IRWXO
+#define S_IROTH 0
+#define S_IWOTH 0
+#define S_IXOTH 0
+#define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)
+#endif
+
+#define S_ISUID 0004000
+#define S_ISGID 0002000
+#define S_ISVTX 0001000
+
+#define WIFEXITED(x) 1
+#define WIFSIGNALED(x) 0
+#define WEXITSTATUS(x) ((x) & 0xff)
+#define WTERMSIG(x) SIGTERM
+
+#ifndef EWOULDBLOCK
+#define EWOULDBLOCK EAGAIN
+#endif
+#ifndef ELOOP
+#define ELOOP EMLINK
+#endif
+#define SHUT_WR SD_SEND
+
+#define SIGHUP 1
+#define SIGQUIT 3
+#define SIGKILL 9
+#define SIGPIPE 13
+#define SIGALRM 14
+#define SIGCHLD 17
+
+#define F_GETFD 1
+#define F_SETFD 2
+#define FD_CLOEXEC 0x1
+
+#if !defined O_CLOEXEC && defined O_NOINHERIT
+#define O_CLOEXEC	O_NOINHERIT
+#endif
+
+#ifndef EAFNOSUPPORT
+#define EAFNOSUPPORT WSAEAFNOSUPPORT
+#endif
+#ifndef ECONNABORTED
+#define ECONNABORTED WSAECONNABORTED
+#endif
+#ifndef ENOTSOCK
+#define ENOTSOCK WSAENOTSOCK
+#endif
+
+struct passwd {
+	char *pw_name;
+	char *pw_gecos;
+	char *pw_dir;
+};
+
+typedef void (__cdecl *sig_handler_t)(int);
+struct sigaction {
+	sig_handler_t sa_handler;
+	unsigned sa_flags;
+};
+#define SA_RESTART 0
+
+struct itimerval {
+	struct timeval it_value, it_interval;
+};
+#define ITIMER_REAL 0
+
+struct utsname {
+	char sysname[16];
+	char nodename[1];
+	char release[16];
+	char version[16];
+	char machine[1];
+};
+
+/*
+ * sanitize preprocessor namespace polluted by Windows headers defining
+ * macros which collide with git local versions
+ */
+#undef HELP_COMMAND /* from winuser.h */
+
+/*
+ * trivial stubs
+ */
+
+static inline int readlink(const char *path UNUSED, char *buf UNUSED, size_t bufsiz UNUSED)
+{ errno = ENOSYS; return -1; }
+static inline int symlink(const char *oldpath UNUSED, const char *newpath UNUSED)
+{ errno = ENOSYS; return -1; }
+static inline int fchmod(int fildes UNUSED, mode_t mode UNUSED)
+{ errno = ENOSYS; return -1; }
+#ifndef __MINGW64_VERSION_MAJOR
+static inline pid_t fork(void)
+{ errno = ENOSYS; return -1; }
+#endif
+static inline unsigned int alarm(unsigned int seconds UNUSED)
+{ return 0; }
+static inline int fsync(int fd)
+{ return _commit(fd); }
+static inline void sync(void)
+{}
+static inline uid_t getuid(void)
+{ return 1; }
+static inline struct passwd *getpwnam(const char *name UNUSED)
+{ return NULL; }
+static inline int fcntl(int fd UNUSED, int cmd, ...)
+{
+	if (cmd == F_GETFD || cmd == F_SETFD)
+		return 0;
+	errno = EINVAL;
+	return -1;
+}
+
+#define sigemptyset(x) (void)0
+static inline int sigaddset(sigset_t *set UNUSED, int signum UNUSED)
+{ return 0; }
+#define SIG_BLOCK 0
+#define SIG_UNBLOCK 0
+static inline int sigprocmask(int how UNUSED, const sigset_t *set UNUSED, sigset_t *oldset UNUSED)
+{ return 0; }
+static inline pid_t getppid(void)
+{ return 1; }
+static inline pid_t getpgid(pid_t pid)
+{ return pid == 0 ? getpid() : pid; }
+static inline pid_t tcgetpgrp(int fd UNUSED)
+{ return getpid(); }
+
+/*
+ * simple adaptors
+ */
+
+int mingw_mkdir(const char *path, int mode);
+#define mkdir mingw_mkdir
+
+#define WNOHANG 1
+pid_t waitpid(pid_t pid, int *status, int options);
+
+#define kill mingw_kill
+int mingw_kill(pid_t pid, int sig);
+
+#define locate_in_PATH mingw_locate_in_PATH
+char *mingw_locate_in_PATH(const char *cmd);
+
+/*
+ * implementations of missing functions
+ */
+
+int pipe(int filedes[2]);
+unsigned int sleep (unsigned int seconds);
+int mkstemp(char *template);
+int gettimeofday(struct timeval *tv, void *tz);
+#ifndef __MINGW64_VERSION_MAJOR
+struct tm *gmtime_r(const time_t *timep, struct tm *result);
+struct tm *localtime_r(const time_t *timep, struct tm *result);
+#endif
+int getpagesize(void);	/* defined in MinGW's libgcc.a */
+struct passwd *getpwuid(uid_t uid);
+int setitimer(int type, struct itimerval *in, struct itimerval *out);
+int sigaction(int sig, struct sigaction *in, struct sigaction *out);
+int link(const char *oldpath, const char *newpath);
+int uname(struct utsname *buf);
+
+/*
+ * replacements of existing functions
+ */
+
+int mingw_unlink(const char *pathname);
+#define unlink mingw_unlink
+
+int mingw_rmdir(const char *path);
+#define rmdir mingw_rmdir
+
+int mingw_open (const char *filename, int oflags, ...);
+#define open mingw_open
+#undef OPEN_RETURNS_EINTR
+
+int mingw_fgetc(FILE *stream);
+#define fgetc mingw_fgetc
+
+FILE *mingw_fopen (const char *filename, const char *otype);
+#define fopen mingw_fopen
+
+FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream);
+#define freopen mingw_freopen
+
+int mingw_fflush(FILE *stream);
+#define fflush mingw_fflush
+
+ssize_t mingw_write(int fd, const void *buf, size_t len);
+#define write mingw_write
+
+int mingw_access(const char *filename, int mode);
+#undef access
+#define access mingw_access
+
+int mingw_chdir(const char *dirname);
+#define chdir mingw_chdir
+
+int mingw_chmod(const char *filename, int mode);
+#define chmod mingw_chmod
+
+char *mingw_mktemp(char *template);
+#define mktemp mingw_mktemp
+
+char *mingw_getcwd(char *pointer, int len);
+#define getcwd mingw_getcwd
+
+#ifdef NO_UNSETENV
+#error "NO_UNSETENV is incompatible with the Windows-specific startup code!"
+#endif
+
+/*
+ * We bind *env() routines (even the mingw_ ones) to private mingw_ versions.
+ * These talk to the CRT using UNICODE/wchar_t, but maintain the original
+ * narrow-char API.
+ *
+ * Note that the MSCRT maintains both ANSI (getenv()) and UNICODE (_wgetenv())
+ * routines and stores both versions of each environment variable in parallel
+ * (and secretly updates both when you set one or the other), but it uses CP_ACP
+ * to do the conversion rather than CP_UTF8.
+ *
+ * Since everything in the git code base is UTF8, we define the mingw_ routines
+ * to access the CRT using the UNICODE routines and manually convert them to
+ * UTF8.  This also avoids round-trip problems.
+ *
+ * This also helps with our linkage, since "_wenviron" is publicly exported
+ * from the CRT.  But to access "_environ" we would have to statically link
+ * to the CRT (/MT).
+ *
+ * We require NO_SETENV (and let gitsetenv() call our mingw_putenv).
+ */
+#define getenv       mingw_getenv
+#define putenv       mingw_putenv
+#define unsetenv     mingw_putenv
+char *mingw_getenv(const char *name);
+int   mingw_putenv(const char *name);
+
+int mingw_gethostname(char *host, int namelen);
+#define gethostname mingw_gethostname
+
+struct hostent *mingw_gethostbyname(const char *host);
+#define gethostbyname mingw_gethostbyname
+
+int mingw_getaddrinfo(const char *node, const char *service,
+		      const struct addrinfo *hints, struct addrinfo **res);
+#define getaddrinfo mingw_getaddrinfo
+
+int mingw_socket(int domain, int type, int protocol);
+#define socket mingw_socket
+
+int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
+#define connect mingw_connect
+
+int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz);
+#define bind mingw_bind
+
+int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int optlen);
+#define setsockopt mingw_setsockopt
+
+int mingw_shutdown(int sockfd, int how);
+#define shutdown mingw_shutdown
+
+int mingw_listen(int sockfd, int backlog);
+#define listen mingw_listen
+
+int mingw_accept(int sockfd, struct sockaddr *sa, socklen_t *sz);
+#define accept mingw_accept
+
+int mingw_rename(const char*, const char*);
+#define rename mingw_rename
+
+#if defined(USE_WIN32_MMAP) || defined(_MSC_VER)
+int mingw_getpagesize(void);
+#define getpagesize mingw_getpagesize
+#endif
+
+int win32_fsync_no_flush(int fd);
+#define fsync_no_flush win32_fsync_no_flush
+
+#define FSYNC_COMPONENTS_PLATFORM_DEFAULT (FSYNC_COMPONENTS_DEFAULT | FSYNC_COMPONENT_LOOSE_OBJECT)
+#define FSYNC_METHOD_DEFAULT (FSYNC_METHOD_BATCH)
+
+struct rlimit {
+	unsigned int rlim_cur;
+};
+#define RLIMIT_NOFILE 0
+
+static inline int getrlimit(int resource, struct rlimit *rlp)
+{
+	if (resource != RLIMIT_NOFILE) {
+		errno = EINVAL;
+		return -1;
+	}
+
+	rlp->rlim_cur = 2048;
+	return 0;
+}
+
+/*
+ * Use mingw specific stat()/lstat()/fstat() implementations on Windows,
+ * including our own struct stat with 64 bit st_size and nanosecond-precision
+ * file times.
+ */
+#ifndef __MINGW64_VERSION_MAJOR
+#define off_t off64_t
+#define lseek _lseeki64
+#ifndef _MSC_VER
+struct timespec {
+	time_t tv_sec;
+	long tv_nsec;
+};
+#endif
+#endif
+
+struct mingw_stat {
+    _dev_t st_dev;
+    _ino_t st_ino;
+    _mode_t st_mode;
+    short st_nlink;
+    short st_uid;
+    short st_gid;
+    _dev_t st_rdev;
+    off64_t st_size;
+    struct timespec st_atim;
+    struct timespec st_mtim;
+    struct timespec st_ctim;
+};
+
+#define st_atime st_atim.tv_sec
+#define st_mtime st_mtim.tv_sec
+#define st_ctime st_ctim.tv_sec
+
+#ifdef stat
+#undef stat
+#endif
+#define stat mingw_stat
+int mingw_lstat(const char *file_name, struct stat *buf);
+int mingw_stat(const char *file_name, struct stat *buf);
+int mingw_fstat(int fd, struct stat *buf);
+#ifdef fstat
+#undef fstat
+#endif
+#define fstat mingw_fstat
+#ifdef lstat
+#undef lstat
+#endif
+#define lstat mingw_lstat
+
+
+int mingw_utime(const char *file_name, const struct utimbuf *times);
+#define utime mingw_utime
+size_t mingw_strftime(char *s, size_t max,
+		   const char *format, const struct tm *tm);
+#define strftime mingw_strftime
+
+pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
+		     const char *dir,
+		     int fhin, int fhout, int fherr);
+int mingw_execvp(const char *cmd, char *const *argv);
+#define execvp mingw_execvp
+int mingw_execv(const char *cmd, char *const *argv);
+#define execv mingw_execv
+
+static inline unsigned int git_ntohl(unsigned int x)
+{ return (unsigned int)ntohl(x); }
+#define ntohl git_ntohl
+
+sig_handler_t mingw_signal(int sig, sig_handler_t handler);
+#define signal mingw_signal
+
+int mingw_raise(int sig);
+#define raise mingw_raise
+
+/*
+ * ANSI emulation wrappers
+ */
+
+int winansi_isatty(int fd);
+#define isatty winansi_isatty
+
+int winansi_dup2(int oldfd, int newfd);
+#define dup2 winansi_dup2
+
+void winansi_init(void);
+HANDLE winansi_get_osfhandle(int fd);
+
+#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 1800)
+#define PRIuMAX "I64u"
+#define PRId64 "I64d"
+#else
+#include <inttypes.h>
+#endif
+
+#endif /* COMPAT_MINGW_POSIX_H */
diff --git a/compat/mingw.h b/compat/mingw.h
index ebfb8ba423b..444daedfa52 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -1,185 +1,10 @@
-#ifdef __MINGW64_VERSION_MAJOR
-#include <stdint.h>
-#include <wchar.h>
-typedef _sigset_t sigset_t;
-#endif
-#include <winsock2.h>
-#include <ws2tcpip.h>
-
-/* MinGW-w64 reports to have flockfile, but it does not actually have it. */
-#ifdef __MINGW64_VERSION_MAJOR
-#undef _POSIX_THREAD_SAFE_FUNCTIONS
-#endif
+#include "mingw-posix.h"
 
 struct config_context;
 int mingw_core_config(const char *var, const char *value,
 		      const struct config_context *ctx, void *cb);
 #define platform_core_config mingw_core_config
 
-/*
- * things that are not available in header files
- */
-
-typedef int uid_t;
-typedef int socklen_t;
-#ifndef __MINGW64_VERSION_MAJOR
-typedef int pid_t;
-#define hstrerror strerror
-#endif
-
-#define S_IFLNK    0120000 /* Symbolic link */
-#define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK)
-#define S_ISSOCK(x) 0
-
-#ifndef S_IRWXG
-#define S_IRGRP 0
-#define S_IWGRP 0
-#define S_IXGRP 0
-#define S_IRWXG (S_IRGRP | S_IWGRP | S_IXGRP)
-#endif
-#ifndef S_IRWXO
-#define S_IROTH 0
-#define S_IWOTH 0
-#define S_IXOTH 0
-#define S_IRWXO (S_IROTH | S_IWOTH | S_IXOTH)
-#endif
-
-#define S_ISUID 0004000
-#define S_ISGID 0002000
-#define S_ISVTX 0001000
-
-#define WIFEXITED(x) 1
-#define WIFSIGNALED(x) 0
-#define WEXITSTATUS(x) ((x) & 0xff)
-#define WTERMSIG(x) SIGTERM
-
-#ifndef EWOULDBLOCK
-#define EWOULDBLOCK EAGAIN
-#endif
-#ifndef ELOOP
-#define ELOOP EMLINK
-#endif
-#define SHUT_WR SD_SEND
-
-#define SIGHUP 1
-#define SIGQUIT 3
-#define SIGKILL 9
-#define SIGPIPE 13
-#define SIGALRM 14
-#define SIGCHLD 17
-
-#define F_GETFD 1
-#define F_SETFD 2
-#define FD_CLOEXEC 0x1
-
-#if !defined O_CLOEXEC && defined O_NOINHERIT
-#define O_CLOEXEC	O_NOINHERIT
-#endif
-
-#ifndef EAFNOSUPPORT
-#define EAFNOSUPPORT WSAEAFNOSUPPORT
-#endif
-#ifndef ECONNABORTED
-#define ECONNABORTED WSAECONNABORTED
-#endif
-#ifndef ENOTSOCK
-#define ENOTSOCK WSAENOTSOCK
-#endif
-
-struct passwd {
-	char *pw_name;
-	char *pw_gecos;
-	char *pw_dir;
-};
-
-typedef void (__cdecl *sig_handler_t)(int);
-struct sigaction {
-	sig_handler_t sa_handler;
-	unsigned sa_flags;
-};
-#define SA_RESTART 0
-
-struct itimerval {
-	struct timeval it_value, it_interval;
-};
-#define ITIMER_REAL 0
-
-struct utsname {
-	char sysname[16];
-	char nodename[1];
-	char release[16];
-	char version[16];
-	char machine[1];
-};
-
-/*
- * sanitize preprocessor namespace polluted by Windows headers defining
- * macros which collide with git local versions
- */
-#undef HELP_COMMAND /* from winuser.h */
-
-/*
- * trivial stubs
- */
-
-static inline int readlink(const char *path UNUSED, char *buf UNUSED, size_t bufsiz UNUSED)
-{ errno = ENOSYS; return -1; }
-static inline int symlink(const char *oldpath UNUSED, const char *newpath UNUSED)
-{ errno = ENOSYS; return -1; }
-static inline int fchmod(int fildes UNUSED, mode_t mode UNUSED)
-{ errno = ENOSYS; return -1; }
-#ifndef __MINGW64_VERSION_MAJOR
-static inline pid_t fork(void)
-{ errno = ENOSYS; return -1; }
-#endif
-static inline unsigned int alarm(unsigned int seconds UNUSED)
-{ return 0; }
-static inline int fsync(int fd)
-{ return _commit(fd); }
-static inline void sync(void)
-{}
-static inline uid_t getuid(void)
-{ return 1; }
-static inline struct passwd *getpwnam(const char *name UNUSED)
-{ return NULL; }
-static inline int fcntl(int fd UNUSED, int cmd, ...)
-{
-	if (cmd == F_GETFD || cmd == F_SETFD)
-		return 0;
-	errno = EINVAL;
-	return -1;
-}
-
-#define sigemptyset(x) (void)0
-static inline int sigaddset(sigset_t *set UNUSED, int signum UNUSED)
-{ return 0; }
-#define SIG_BLOCK 0
-#define SIG_UNBLOCK 0
-static inline int sigprocmask(int how UNUSED, const sigset_t *set UNUSED, sigset_t *oldset UNUSED)
-{ return 0; }
-static inline pid_t getppid(void)
-{ return 1; }
-static inline pid_t getpgid(pid_t pid)
-{ return pid == 0 ? getpid() : pid; }
-static inline pid_t tcgetpgrp(int fd UNUSED)
-{ return getpid(); }
-
-/*
- * simple adaptors
- */
-
-int mingw_mkdir(const char *path, int mode);
-#define mkdir mingw_mkdir
-
-#define WNOHANG 1
-pid_t waitpid(pid_t pid, int *status, int options);
-
-#define kill mingw_kill
-int mingw_kill(pid_t pid, int sig);
-
-#define locate_in_PATH mingw_locate_in_PATH
-char *mingw_locate_in_PATH(const char *cmd);
-
 #ifndef NO_OPENSSL
 #include <openssl/ssl.h>
 static inline int mingw_SSL_set_fd(SSL *ssl, int fd)
@@ -201,249 +26,6 @@ static inline int mingw_SSL_set_wfd(SSL *ssl, int fd)
 #define SSL_set_wfd mingw_SSL_set_wfd
 #endif
 
-/*
- * implementations of missing functions
- */
-
-int pipe(int filedes[2]);
-unsigned int sleep (unsigned int seconds);
-int mkstemp(char *template);
-int gettimeofday(struct timeval *tv, void *tz);
-#ifndef __MINGW64_VERSION_MAJOR
-struct tm *gmtime_r(const time_t *timep, struct tm *result);
-struct tm *localtime_r(const time_t *timep, struct tm *result);
-#endif
-int getpagesize(void);	/* defined in MinGW's libgcc.a */
-struct passwd *getpwuid(uid_t uid);
-int setitimer(int type, struct itimerval *in, struct itimerval *out);
-int sigaction(int sig, struct sigaction *in, struct sigaction *out);
-int link(const char *oldpath, const char *newpath);
-int uname(struct utsname *buf);
-
-/*
- * replacements of existing functions
- */
-
-int mingw_unlink(const char *pathname);
-#define unlink mingw_unlink
-
-int mingw_rmdir(const char *path);
-#define rmdir mingw_rmdir
-
-int mingw_open (const char *filename, int oflags, ...);
-#define open mingw_open
-#undef OPEN_RETURNS_EINTR
-
-int mingw_fgetc(FILE *stream);
-#define fgetc mingw_fgetc
-
-FILE *mingw_fopen (const char *filename, const char *otype);
-#define fopen mingw_fopen
-
-FILE *mingw_freopen (const char *filename, const char *otype, FILE *stream);
-#define freopen mingw_freopen
-
-int mingw_fflush(FILE *stream);
-#define fflush mingw_fflush
-
-ssize_t mingw_write(int fd, const void *buf, size_t len);
-#define write mingw_write
-
-int mingw_access(const char *filename, int mode);
-#undef access
-#define access mingw_access
-
-int mingw_chdir(const char *dirname);
-#define chdir mingw_chdir
-
-int mingw_chmod(const char *filename, int mode);
-#define chmod mingw_chmod
-
-char *mingw_mktemp(char *template);
-#define mktemp mingw_mktemp
-
-char *mingw_getcwd(char *pointer, int len);
-#define getcwd mingw_getcwd
-
-#ifdef NO_UNSETENV
-#error "NO_UNSETENV is incompatible with the Windows-specific startup code!"
-#endif
-
-/*
- * We bind *env() routines (even the mingw_ ones) to private mingw_ versions.
- * These talk to the CRT using UNICODE/wchar_t, but maintain the original
- * narrow-char API.
- *
- * Note that the MSCRT maintains both ANSI (getenv()) and UNICODE (_wgetenv())
- * routines and stores both versions of each environment variable in parallel
- * (and secretly updates both when you set one or the other), but it uses CP_ACP
- * to do the conversion rather than CP_UTF8.
- *
- * Since everything in the git code base is UTF8, we define the mingw_ routines
- * to access the CRT using the UNICODE routines and manually convert them to
- * UTF8.  This also avoids round-trip problems.
- *
- * This also helps with our linkage, since "_wenviron" is publicly exported
- * from the CRT.  But to access "_environ" we would have to statically link
- * to the CRT (/MT).
- *
- * We require NO_SETENV (and let gitsetenv() call our mingw_putenv).
- */
-#define getenv       mingw_getenv
-#define putenv       mingw_putenv
-#define unsetenv     mingw_putenv
-char *mingw_getenv(const char *name);
-int   mingw_putenv(const char *name);
-
-int mingw_gethostname(char *host, int namelen);
-#define gethostname mingw_gethostname
-
-struct hostent *mingw_gethostbyname(const char *host);
-#define gethostbyname mingw_gethostbyname
-
-int mingw_getaddrinfo(const char *node, const char *service,
-		      const struct addrinfo *hints, struct addrinfo **res);
-#define getaddrinfo mingw_getaddrinfo
-
-int mingw_socket(int domain, int type, int protocol);
-#define socket mingw_socket
-
-int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
-#define connect mingw_connect
-
-int mingw_bind(int sockfd, struct sockaddr *sa, size_t sz);
-#define bind mingw_bind
-
-int mingw_setsockopt(int sockfd, int lvl, int optname, void *optval, int optlen);
-#define setsockopt mingw_setsockopt
-
-int mingw_shutdown(int sockfd, int how);
-#define shutdown mingw_shutdown
-
-int mingw_listen(int sockfd, int backlog);
-#define listen mingw_listen
-
-int mingw_accept(int sockfd, struct sockaddr *sa, socklen_t *sz);
-#define accept mingw_accept
-
-int mingw_rename(const char*, const char*);
-#define rename mingw_rename
-
-#if defined(USE_WIN32_MMAP) || defined(_MSC_VER)
-int mingw_getpagesize(void);
-#define getpagesize mingw_getpagesize
-#endif
-
-int win32_fsync_no_flush(int fd);
-#define fsync_no_flush win32_fsync_no_flush
-
-#define FSYNC_COMPONENTS_PLATFORM_DEFAULT (FSYNC_COMPONENTS_DEFAULT | FSYNC_COMPONENT_LOOSE_OBJECT)
-#define FSYNC_METHOD_DEFAULT (FSYNC_METHOD_BATCH)
-
-struct rlimit {
-	unsigned int rlim_cur;
-};
-#define RLIMIT_NOFILE 0
-
-static inline int getrlimit(int resource, struct rlimit *rlp)
-{
-	if (resource != RLIMIT_NOFILE) {
-		errno = EINVAL;
-		return -1;
-	}
-
-	rlp->rlim_cur = 2048;
-	return 0;
-}
-
-/*
- * Use mingw specific stat()/lstat()/fstat() implementations on Windows,
- * including our own struct stat with 64 bit st_size and nanosecond-precision
- * file times.
- */
-#ifndef __MINGW64_VERSION_MAJOR
-#define off_t off64_t
-#define lseek _lseeki64
-#ifndef _MSC_VER
-struct timespec {
-	time_t tv_sec;
-	long tv_nsec;
-};
-#endif
-#endif
-
-struct mingw_stat {
-    _dev_t st_dev;
-    _ino_t st_ino;
-    _mode_t st_mode;
-    short st_nlink;
-    short st_uid;
-    short st_gid;
-    _dev_t st_rdev;
-    off64_t st_size;
-    struct timespec st_atim;
-    struct timespec st_mtim;
-    struct timespec st_ctim;
-};
-
-#define st_atime st_atim.tv_sec
-#define st_mtime st_mtim.tv_sec
-#define st_ctime st_ctim.tv_sec
-
-#ifdef stat
-#undef stat
-#endif
-#define stat mingw_stat
-int mingw_lstat(const char *file_name, struct stat *buf);
-int mingw_stat(const char *file_name, struct stat *buf);
-int mingw_fstat(int fd, struct stat *buf);
-#ifdef fstat
-#undef fstat
-#endif
-#define fstat mingw_fstat
-#ifdef lstat
-#undef lstat
-#endif
-#define lstat mingw_lstat
-
-
-int mingw_utime(const char *file_name, const struct utimbuf *times);
-#define utime mingw_utime
-size_t mingw_strftime(char *s, size_t max,
-		   const char *format, const struct tm *tm);
-#define strftime mingw_strftime
-
-pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
-		     const char *dir,
-		     int fhin, int fhout, int fherr);
-int mingw_execvp(const char *cmd, char *const *argv);
-#define execvp mingw_execvp
-int mingw_execv(const char *cmd, char *const *argv);
-#define execv mingw_execv
-
-static inline unsigned int git_ntohl(unsigned int x)
-{ return (unsigned int)ntohl(x); }
-#define ntohl git_ntohl
-
-sig_handler_t mingw_signal(int sig, sig_handler_t handler);
-#define signal mingw_signal
-
-int mingw_raise(int sig);
-#define raise mingw_raise
-
-/*
- * ANSI emulation wrappers
- */
-
-int winansi_isatty(int fd);
-#define isatty winansi_isatty
-
-int winansi_dup2(int oldfd, int newfd);
-#define dup2 winansi_dup2
-
-void winansi_init(void);
-HANDLE winansi_get_osfhandle(int fd);
-
 /*
  * git specific compatibility
  */
@@ -457,12 +39,6 @@ static inline void convert_slashes(char *path)
 #define PATH_SEP ';'
 char *mingw_query_user_email(void);
 #define query_user_email mingw_query_user_email
-#if !defined(__MINGW64_VERSION_MAJOR) && (!defined(_MSC_VER) || _MSC_VER < 1800)
-#define PRIuMAX "I64u"
-#define PRId64 "I64d"
-#else
-#include <inttypes.h>
-#endif
 
 /**
  * Verifies that the specified path is owned by the user running the
diff --git a/compat/msvc-posix.h b/compat/msvc-posix.h
new file mode 100644
index 00000000000..c500b8b4aaf
--- /dev/null
+++ b/compat/msvc-posix.h
@@ -0,0 +1,33 @@
+#ifndef COMPAT_MSVC_POSIX_H
+#define COMPAT_MSVC_POSIX_H
+
+#include <direct.h>
+#include <process.h>
+#include <malloc.h>
+#include <io.h>
+
+#pragma warning(disable: 4018) /* signed/unsigned comparison */
+#pragma warning(disable: 4244) /* type conversion, possible loss of data */
+#pragma warning(disable: 4090) /* 'function' : different 'const' qualifiers (ALLOC_GROW etc.)*/
+
+/* porting function */
+#define inline __inline
+#define __inline__ __inline
+#define __attribute__(x)
+#define strcasecmp   _stricmp
+#define strncasecmp  _strnicmp
+#define ftruncate    _chsize
+#define strtoull     _strtoui64
+#define strtoll      _strtoi64
+
+#undef ERROR
+
+#define ftello _ftelli64
+
+typedef int sigset_t;
+/* open for reading, writing, or both (not in fcntl.h) */
+#define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
+
+#include "mingw-posix.h"
+
+#endif /* COMPAT_MSVC_POSIX_H */
diff --git a/compat/msvc.h b/compat/msvc.h
index 1d7a8c61456..2b87c0a7c70 100644
--- a/compat/msvc.h
+++ b/compat/msvc.h
@@ -1,33 +1,7 @@
 #ifndef __MSVC__HEAD
 #define __MSVC__HEAD
 
-#include <direct.h>
-#include <process.h>
-#include <malloc.h>
-#include <io.h>
-
-#pragma warning(disable: 4018) /* signed/unsigned comparison */
-#pragma warning(disable: 4244) /* type conversion, possible loss of data */
-#pragma warning(disable: 4090) /* 'function' : different 'const' qualifiers (ALLOC_GROW etc.)*/
-
-/* porting function */
-#define inline __inline
-#define __inline__ __inline
-#define __attribute__(x)
-#define strcasecmp   _stricmp
-#define strncasecmp  _strnicmp
-#define ftruncate    _chsize
-#define strtoull     _strtoui64
-#define strtoll      _strtoi64
-
-#undef ERROR
-
-#define ftello _ftelli64
-
-typedef int sigset_t;
-/* open for reading, writing, or both (not in fcntl.h) */
-#define O_ACCMODE     (_O_RDONLY | _O_WRONLY | _O_RDWR)
-
-#include "compat/mingw.h"
+#include "msvc-posix.h"
+#include "mingw.h"
 
 #endif
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 16/18] git-compat-util.h: split out POSIX-emulating bits

The "git-compat-util.h" header is a treasure trove of various bits and pieces used throughout the project. It basically mixes two different things into one:

  - Providing a POSIX-like interface even on platforms that aren't
    POSIX-compliant.
  - Providing low-level functionality that is specific to Git.

This intermixing is a bit of a problem for the reftable library as we don't want to recreate the POSIX-like interface there. But neither do we want to pull in the Git-specific functionality, as it is otherwise quite easy to start depending on the Git codebase again.

Split out a new header "compat/posix.h" that only contains the bits and pieces relevant for the emulation of POSIX, which we will start using in the next commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 compat/posix.h    | 541 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 git-compat-util.h | 531 +----------------------------------------------------
 2 files changed, 543 insertions(+), 529 deletions(-)
Show changes to 2 files +543 −529

compat/posix.h, git-compat-util.h

diff --git a/compat/posix.h b/compat/posix.h
new file mode 100644
index 00000000000..f4c71f9427d
--- /dev/null
+++ b/compat/posix.h
@@ -0,0 +1,541 @@
+#ifndef COMPAT_POSIX_H
+#define COMPAT_POSIX_H
+
+#define _FILE_OFFSET_BITS 64
+
+/*
+ * Derived from Linux "Features Test Macro" header
+ * Convenience macros to test the versions of gcc (or
+ * a compatible compiler).
+ * Use them like this:
+ *  #if GIT_GNUC_PREREQ (2,8)
+ *   ... code requiring gcc 2.8 or later ...
+ *  #endif
+ *
+ * This macro of course is not part of POSIX, but we need it for the UNUSED
+ * macro which is used by some of our POSIX compatibility wrappers.
+*/
+#if defined(__GNUC__) && defined(__GNUC_MINOR__)
+# define GIT_GNUC_PREREQ(maj, min) \
+	((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
+#else
+ #define GIT_GNUC_PREREQ(maj, min) 0
+#endif
+
+/*
+ * UNUSED marks a function parameter that is always unused.  It also
+ * can be used to annotate a function, a variable, or a type that is
+ * always unused.
+ *
+ * A callback interface may dictate that a function accepts a
+ * parameter at that position, but the implementation of the function
+ * may not need to use the parameter.  In such a case, mark the parameter
+ * with UNUSED.
+ *
+ * When a parameter may be used or unused, depending on conditional
+ * compilation, consider using MAYBE_UNUSED instead.
+ */
+#if GIT_GNUC_PREREQ(4, 5)
+#define UNUSED __attribute__((unused)) \
+	__attribute__((deprecated ("parameter declared as UNUSED")))
+#elif defined(__GNUC__)
+#define UNUSED __attribute__((unused)) \
+	__attribute__((deprecated))
+#else
+#define UNUSED
+#endif
+
+#ifdef __MINGW64__
+#define _POSIX_C_SOURCE 1
+#elif defined(__sun__)
+ /*
+  * On Solaris, when _XOPEN_EXTENDED is set, its header file
+  * forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE
+  * setting to say we are XPG5 or XPG6.  Also on Solaris,
+  * XPG6 programs must be compiled with a c99 compiler, while
+  * non XPG6 programs must be compiled with a pre-c99 compiler.
+  */
+# if __STDC_VERSION__ - 0 >= 199901L
+# define _XOPEN_SOURCE 600
+# else
+# define _XOPEN_SOURCE 500
+# endif
+#elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
+      !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
+      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__) && \
+      !defined(__CYGWIN__)
+#define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
+#define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
+#endif
+#define _ALL_SOURCE 1
+#define _GNU_SOURCE 1
+#define _BSD_SOURCE 1
+#define _DEFAULT_SOURCE 1
+#define _NETBSD_SOURCE 1
+#define _SGI_SOURCE 1
+
+#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
+# if !defined(_WIN32_WINNT)
+#  define _WIN32_WINNT 0x0600
+# endif
+#define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
+#include <winsock2.h>
+#ifndef NO_UNIX_SOCKETS
+#include <afunix.h>
+#endif
+#include <windows.h>
+#define GIT_WINDOWS_NATIVE
+#endif
+
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <string.h>
+#ifdef HAVE_STRINGS_H
+#include <strings.h> /* for strcasecmp() */
+#endif
+#include <errno.h>
+#include <limits.h>
+#include <locale.h>
+#ifdef NEEDS_SYS_PARAM_H
+#include <sys/param.h>
+#endif
+#include <sys/types.h>
+#include <dirent.h>
+#include <sys/time.h>
+#include <time.h>
+#include <signal.h>
+#include <assert.h>
+#include <regex.h>
+#include <utime.h>
+#include <syslog.h>
+#if !defined(NO_POLL_H)
+#include <poll.h>
+#elif !defined(NO_SYS_POLL_H)
+#include <sys/poll.h>
+#else
+/* Pull the compat stuff */
+#include <poll.h>
+#endif
+#ifdef HAVE_BSD_SYSCTL
+#include <sys/sysctl.h>
+#endif
+
+#if defined(__MINGW32__)
+#include "mingw-posix.h"
+#elif defined(_MSC_VER)
+#include "msvc-posix.h"
+#else
+#include <sys/utsname.h>
+#include <sys/wait.h>
+#include <sys/resource.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
+#include <sys/statvfs.h>
+#include <termios.h>
+#ifndef NO_SYS_SELECT_H
+#include <sys/select.h>
+#endif
+#include <netinet/in.h>
+#include <netinet/tcp.h>
+#include <arpa/inet.h>
+#include <netdb.h>
+#include <pwd.h>
+#include <sys/un.h>
+#ifndef NO_INTTYPES_H
+#include <inttypes.h>
+#else
+#include <stdint.h>
+#endif
+#ifdef HAVE_ARC4RANDOM_LIBBSD
+#include <bsd/stdlib.h>
+#endif
+#ifdef HAVE_GETRANDOM
+#include <sys/random.h>
+#endif
+#ifdef NO_INTPTR_T
+/*
+ * On I16LP32, ILP32 and LP64 "long" is the safe bet, however
+ * on LLP86, IL33LLP64 and P64 it needs to be "long long",
+ * while on IP16 and IP16L32 it is "int" (resp. "short")
+ * Size needs to match (or exceed) 'sizeof(void *)'.
+ * We can't take "long long" here as not everybody has it.
+ */
+typedef long intptr_t;
+typedef unsigned long uintptr_t;
+#endif
+#undef _ALL_SOURCE /* AIX 5.3L defines a struct list with _ALL_SOURCE. */
+#include <grp.h>
+#define _ALL_SOURCE 1
+#endif
+
+#ifdef MKDIR_WO_TRAILING_SLASH
+#define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b))
+int compat_mkdir_wo_trailing_slash(const char*, mode_t);
+#endif
+
+#ifdef time
+#undef time
+#endif
+static inline time_t git_time(time_t *tloc)
+{
+	struct timeval tv;
+
+	/*
+	 * Avoid time(NULL), which can disagree with gettimeofday(2)
+	 * and filesystem timestamps.
+	 */
+	gettimeofday(&tv, NULL);
+
+	if (tloc)
+		*tloc = tv.tv_sec;
+	return tv.tv_sec;
+}
+#define time git_time
+
+#ifdef NO_STRUCT_ITIMERVAL
+struct itimerval {
+	struct timeval it_interval;
+	struct timeval it_value;
+};
+#endif
+
+#ifdef NO_SETITIMER
+static inline int git_setitimer(int which UNUSED,
+				const struct itimerval *value UNUSED,
+				struct itimerval *newvalue UNUSED) {
+	return 0; /* pretend success */
+}
+#undef setitimer
+#define setitimer(which,value,ovalue) git_setitimer(which,value,ovalue)
+#endif
+
+#ifndef NO_LIBGEN_H
+#include <libgen.h>
+#else
+#define basename gitbasename
+char *gitbasename(char *);
+#define dirname gitdirname
+char *gitdirname(char *);
+#endif
+
+#ifndef NO_ICONV
+#include <iconv.h>
+#endif
+
+/* On most systems <netdb.h> would have given us this, but
+ * not on some systems (e.g. z/OS).
+ */
+#ifndef NI_MAXHOST
+#define NI_MAXHOST 1025
+#endif
+
+#ifndef NI_MAXSERV
+#define NI_MAXSERV 32
+#endif
+
+/* On most systems <limits.h> would have given us this, but
+ * not on some systems (e.g. GNU/Hurd).
+ */
+#ifndef PATH_MAX
+#define PATH_MAX 4096
+#endif
+
+#ifndef NAME_MAX
+#define NAME_MAX 255
+#endif
+
+typedef uintmax_t timestamp_t;
+#define PRItime PRIuMAX
+#define parse_timestamp strtoumax
+#define TIME_MAX UINTMAX_MAX
+#define TIME_MIN 0
+
+int lstat_cache_aware_rmdir(const char *path);
+#if !defined(__MINGW32__) && !defined(_MSC_VER)
+#define rmdir lstat_cache_aware_rmdir
+#endif
+
+#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
+
+#ifndef PROT_READ
+#define PROT_READ 1
+#define PROT_WRITE 2
+#define MAP_PRIVATE 1
+#endif
+
+#define mmap git_mmap
+#define munmap git_munmap
+void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
+int git_munmap(void *start, size_t length);
+
+#else /* NO_MMAP || USE_WIN32_MMAP */
+
+#include <sys/mman.h>
+
+#endif /* NO_MMAP || USE_WIN32_MMAP */
+
+#ifndef MAP_FAILED
+#define MAP_FAILED ((void *)-1)
+#endif
+
+#ifdef NEEDS_MODE_TRANSLATION
+#undef S_IFMT
+#undef S_IFREG
+#undef S_IFDIR
+#undef S_IFLNK
+#undef S_IFBLK
+#undef S_IFCHR
+#undef S_IFIFO
+#undef S_IFSOCK
+#define S_IFMT   0170000
+#define S_IFREG  0100000
+#define S_IFDIR  0040000
+#define S_IFLNK  0120000
+#define S_IFBLK  0060000
+#define S_IFCHR  0020000
+#define S_IFIFO  0010000
+#define S_IFSOCK 0140000
+#ifdef stat
+#undef stat
+#endif
+#define stat(path, buf) git_stat(path, buf)
+int git_stat(const char *, struct stat *);
+#ifdef fstat
+#undef fstat
+#endif
+#define fstat(fd, buf) git_fstat(fd, buf)
+int git_fstat(int, struct stat *);
+#ifdef lstat
+#undef lstat
+#endif
+#define lstat(path, buf) git_lstat(path, buf)
+int git_lstat(const char *, struct stat *);
+#endif
+
+#ifdef NO_PREAD
+#define pread git_pread
+ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
+#endif
+
+#ifdef NO_SETENV
+#define setenv gitsetenv
+int gitsetenv(const char *, const char *, int);
+#endif
+
+#ifdef NO_MKDTEMP
+#define mkdtemp gitmkdtemp
+char *gitmkdtemp(char *);
+#endif
+
+#ifdef NO_UNSETENV
+#define unsetenv gitunsetenv
+int gitunsetenv(const char *);
+#endif
+
+#ifdef NO_STRCASESTR
+#define strcasestr gitstrcasestr
+char *gitstrcasestr(const char *haystack, const char *needle);
+#endif
+
+#ifdef NO_STRLCPY
+#define strlcpy gitstrlcpy
+size_t gitstrlcpy(char *, const char *, size_t);
+#endif
+
+#ifdef NO_STRTOUMAX
+#define strtoumax gitstrtoumax
+uintmax_t gitstrtoumax(const char *, char **, int);
+#define strtoimax gitstrtoimax
+intmax_t gitstrtoimax(const char *, char **, int);
+#endif
+
+#ifdef NO_HSTRERROR
+#define hstrerror githstrerror
+const char *githstrerror(int herror);
+#endif
+
+#ifdef NO_MEMMEM
+#define memmem gitmemmem
+void *gitmemmem(const void *haystack, size_t haystacklen,
+		const void *needle, size_t needlelen);
+#endif
+
+#ifdef OVERRIDE_STRDUP
+#ifdef strdup
+#undef strdup
+#endif
+#define strdup gitstrdup
+char *gitstrdup(const char *s);
+#endif
+
+#ifdef NO_GETPAGESIZE
+#define getpagesize() sysconf(_SC_PAGESIZE)
+#endif
+
+#ifndef O_CLOEXEC
+#define O_CLOEXEC 0
+#endif
+
+#ifdef FREAD_READS_DIRECTORIES
+# if !defined(SUPPRESS_FOPEN_REDEFINITION)
+#  ifdef fopen
+#   undef fopen
+#  endif
+#  define fopen(a,b) git_fopen(a,b)
+# endif
+FILE *git_fopen(const char*, const char*);
+#endif
+
+#ifdef SNPRINTF_RETURNS_BOGUS
+#ifdef snprintf
+#undef snprintf
+#endif
+#define snprintf git_snprintf
+int git_snprintf(char *str, size_t maxsize,
+		 const char *format, ...);
+#ifdef vsnprintf
+#undef vsnprintf
+#endif
+#define vsnprintf git_vsnprintf
+int git_vsnprintf(char *str, size_t maxsize,
+		  const char *format, va_list ap);
+#endif
+
+#ifdef OPEN_RETURNS_EINTR
+#undef open
+#define open git_open_with_retry
+int git_open_with_retry(const char *path, int flag, ...);
+#endif
+
+#ifdef __GLIBC_PREREQ
+#if __GLIBC_PREREQ(2, 1)
+#define HAVE_STRCHRNUL
+#endif
+#endif
+
+#ifndef HAVE_STRCHRNUL
+#define strchrnul gitstrchrnul
+static inline char *gitstrchrnul(const char *s, int c)
+{
+	while (*s && *s != c)
+		s++;
+	return (char *)s;
+}
+#endif
+
+#ifdef NO_INET_PTON
+int inet_pton(int af, const char *src, void *dst);
+#endif
+
+#ifdef NO_INET_NTOP
+const char *inet_ntop(int af, const void *src, char *dst, size_t size);
+#endif
+
+#ifdef NO_PTHREADS
+#define atexit git_atexit
+int git_atexit(void (*handler)(void));
+#endif
+
+#ifndef HOST_NAME_MAX
+#define HOST_NAME_MAX 256
+#endif
+
+#include "../sane-ctype.h"
+
+void git_stable_qsort(void *base, size_t nmemb, size_t size,
+		      int(*compar)(const void *, const void *));
+#ifdef INTERNAL_QSORT
+#define qsort git_stable_qsort
+#endif
+
+#define QSORT(base, n, compar) sane_qsort((base), (n), sizeof(*(base)), compar)
+static inline void sane_qsort(void *base, size_t nmemb, size_t size,
+			      int(*compar)(const void *, const void *))
+{
+	if (nmemb > 1)
+		qsort(base, nmemb, size, compar);
+}
+
+#define STABLE_QSORT(base, n, compar) \
+	git_stable_qsort((base), (n), sizeof(*(base)), compar)
+
+#ifndef HAVE_ISO_QSORT_S
+int git_qsort_s(void *base, size_t nmemb, size_t size,
+		int (*compar)(const void *, const void *, void *), void *ctx);
+#define qsort_s git_qsort_s
+#endif
+
+#define QSORT_S(base, n, compar, ctx) do {			\
+	if (qsort_s((base), (n), sizeof(*(base)), compar, ctx))	\
+		BUG("qsort_s() failed");			\
+} while (0)
+
+#ifdef NO_NSEC
+#undef USE_NSEC
+#define ST_CTIME_NSEC(st) 0
+#define ST_MTIME_NSEC(st) 0
+#else
+#ifdef USE_ST_TIMESPEC
+#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))
+#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))
+#else
+#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
+#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
+#endif
+#endif
+
+#ifndef va_copy
+/*
+ * Since an obvious implementation of va_list would be to make it a
+ * pointer into the stack frame, a simple assignment will work on
+ * many systems.  But let's try to be more portable.
+ */
+#ifdef __va_copy
+#define va_copy(dst, src) __va_copy(dst, src)
+#else
+#define va_copy(dst, src) ((dst) = (src))
+#endif
+#endif
+
+#ifndef _POSIX_THREAD_SAFE_FUNCTIONS
+static inline void git_flockfile(FILE *fh UNUSED)
+{
+	; /* nothing */
+}
+static inline void git_funlockfile(FILE *fh UNUSED)
+{
+	; /* nothing */
+}
+#undef flockfile
+#undef funlockfile
+#undef getc_unlocked
+#define flockfile(fh) git_flockfile(fh)
+#define funlockfile(fh) git_funlockfile(fh)
+#define getc_unlocked(fh) getc(fh)
+#endif
+
+#ifdef FILENO_IS_A_MACRO
+int git_fileno(FILE *stream);
+# ifndef COMPAT_CODE_FILENO
+#  undef fileno
+#  define fileno(p) git_fileno(p)
+# endif
+#endif
+
+#ifdef NEED_ACCESS_ROOT_HANDLER
+int git_access(const char *path, int mode);
+# ifndef COMPAT_CODE_ACCESS
+#  ifdef access
+#  undef access
+#  endif
+#  define access(path, mode) git_access(path, mode)
+# endif
+#endif
+
+#endif /* COMPAT_POSIX_H */
diff --git a/git-compat-util.h b/git-compat-util.h
index e123288e8f1..03e2ba59d3e 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -23,26 +23,9 @@
 #include <crtdbg.h>
 #endif
 
-struct strbuf;
-
-
-#define _FILE_OFFSET_BITS 64
+#include "compat/posix.h"
 
-
-/* Derived from Linux "Features Test Macro" header
- * Convenience macros to test the versions of gcc (or
- * a compatible compiler).
- * Use them like this:
- *  #if GIT_GNUC_PREREQ (2,8)
- *   ... code requiring gcc 2.8 or later ...
- *  #endif
-*/
-#if defined(__GNUC__) && defined(__GNUC_MINOR__)
-# define GIT_GNUC_PREREQ(maj, min) \
-	((__GNUC__ << 16) + __GNUC_MINOR__ >= ((maj) << 16) + (min))
-#else
- #define GIT_GNUC_PREREQ(maj, min) 0
-#endif
+struct strbuf;
 
 #if defined(__GNUC__) || defined(__clang__)
 #  define PRAGMA(pragma)           _Pragma(#pragma)
@@ -176,71 +159,6 @@ DISABLE_WARNING(-Wsign-compare)
 /* Approximation of the length of the decimal representation of this type. */
 #define decimal_length(x)	((int)(sizeof(x) * 2.56 + 0.5) + 1)
 
-#ifdef __MINGW64__
-#define _POSIX_C_SOURCE 1
-#elif defined(__sun__)
- /*
-  * On Solaris, when _XOPEN_EXTENDED is set, its header file
-  * forces the programs to be XPG4v2, defeating any _XOPEN_SOURCE
-  * setting to say we are XPG5 or XPG6.  Also on Solaris,
-  * XPG6 programs must be compiled with a c99 compiler, while
-  * non XPG6 programs must be compiled with a pre-c99 compiler.
-  */
-# if __STDC_VERSION__ - 0 >= 199901L
-# define _XOPEN_SOURCE 600
-# else
-# define _XOPEN_SOURCE 500
-# endif
-#elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
-      !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
-      !defined(__TANDEM) && !defined(__QNX__) && !defined(__MirBSD__) && \
-      !defined(__CYGWIN__)
-#define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
-#define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
-#endif
-#define _ALL_SOURCE 1
-#define _GNU_SOURCE 1
-#define _BSD_SOURCE 1
-#define _DEFAULT_SOURCE 1
-#define _NETBSD_SOURCE 1
-#define _SGI_SOURCE 1
-
-/*
- * UNUSED marks a function parameter that is always unused.  It also
- * can be used to annotate a function, a variable, or a type that is
- * always unused.
- *
- * A callback interface may dictate that a function accepts a
- * parameter at that position, but the implementation of the function
- * may not need to use the parameter.  In such a case, mark the parameter
- * with UNUSED.
- *
- * When a parameter may be used or unused, depending on conditional
- * compilation, consider using MAYBE_UNUSED instead.
- */
-#if GIT_GNUC_PREREQ(4, 5)
-#define UNUSED __attribute__((unused)) \
-	__attribute__((deprecated ("parameter declared as UNUSED")))
-#elif defined(__GNUC__)
-#define UNUSED __attribute__((unused)) \
-	__attribute__((deprecated))
-#else
-#define UNUSED
-#endif
-
-#if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
-# if !defined(_WIN32_WINNT)
-#  define _WIN32_WINNT 0x0600
-# endif
-#define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
-#include <winsock2.h>
-#ifndef NO_UNIX_SOCKETS
-#include <afunix.h>
-#endif
-#include <windows.h>
-#define GIT_WINDOWS_NATIVE
-#endif
-
 #if defined(NO_UNIX_SOCKETS) || !defined(GIT_WINDOWS_NATIVE)
 static inline int _have_unix_sockets(void)
 {
@@ -253,45 +171,6 @@ static inline int _have_unix_sockets(void)
 #define have_unix_sockets _have_unix_sockets
 #endif
 
-#include <unistd.h>
-#include <stdio.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <stdarg.h>
-#include <stdbool.h>
-#include <string.h>
-#ifdef HAVE_STRINGS_H
-#include <strings.h> /* for strcasecmp() */
-#endif
-#include <errno.h>
-#include <limits.h>
-#include <locale.h>
-#ifdef NEEDS_SYS_PARAM_H
-#include <sys/param.h>
-#endif
-#include <sys/types.h>
-#include <dirent.h>
-#include <sys/time.h>
-#include <time.h>
-#include <signal.h>
-#include <assert.h>
-#include <regex.h>
-#include <utime.h>
-#include <syslog.h>
-#if !defined(NO_POLL_H)
-#include <poll.h>
-#elif !defined(NO_SYS_POLL_H)
-#include <sys/poll.h>
-#else
-/* Pull the compat stuff */
-#include <poll.h>
-#endif
-#ifdef HAVE_BSD_SYSCTL
-#include <sys/sysctl.h>
-#endif
-
 /* Used by compat/win32/path-utils.h, and more */
 static inline int is_xplatform_dir_sep(int c)
 {
@@ -308,48 +187,6 @@ static inline int is_xplatform_dir_sep(int c)
 #elif defined(_MSC_VER)
 #include "compat/win32/path-utils.h"
 #include "compat/msvc.h"
-#else
-#include <sys/utsname.h>
-#include <sys/wait.h>
-#include <sys/resource.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <sys/statvfs.h>
-#include <termios.h>
-#ifndef NO_SYS_SELECT_H
-#include <sys/select.h>
-#endif
-#include <netinet/in.h>
-#include <netinet/tcp.h>
-#include <arpa/inet.h>
-#include <netdb.h>
-#include <pwd.h>
-#include <sys/un.h>
-#ifndef NO_INTTYPES_H
-#include <inttypes.h>
-#else
-#include <stdint.h>
-#endif
-#ifdef HAVE_ARC4RANDOM_LIBBSD
-#include <bsd/stdlib.h>
-#endif
-#ifdef HAVE_GETRANDOM
-#include <sys/random.h>
-#endif
-#ifdef NO_INTPTR_T
-/*
- * On I16LP32, ILP32 and LP64 "long" is the safe bet, however
- * on LLP86, IL33LLP64 and P64 it needs to be "long long",
- * while on IP16 and IP16L32 it is "int" (resp. "short")
- * Size needs to match (or exceed) 'sizeof(void *)'.
- * We can't take "long long" here as not everybody has it.
- */
-typedef long intptr_t;
-typedef unsigned long uintptr_t;
-#endif
-#undef _ALL_SOURCE /* AIX 5.3L defines a struct list with _ALL_SOURCE. */
-#include <grp.h>
-#define _ALL_SOURCE 1
 #endif
 
 /* used on Mac OS X */
@@ -370,60 +207,6 @@ static inline const char *precompose_string_if_needed(const char *in)
 #define probe_utf8_pathname_composition()
 #endif
 
-#ifdef MKDIR_WO_TRAILING_SLASH
-#define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b))
-int compat_mkdir_wo_trailing_slash(const char*, mode_t);
-#endif
-
-#ifdef time
-#undef time
-#endif
-static inline time_t git_time(time_t *tloc)
-{
-	struct timeval tv;
-
-	/*
-	 * Avoid time(NULL), which can disagree with gettimeofday(2)
-	 * and filesystem timestamps.
-	 */
-	gettimeofday(&tv, NULL);
-
-	if (tloc)
-		*tloc = tv.tv_sec;
-	return tv.tv_sec;
-}
-#define time git_time
-
-#ifdef NO_STRUCT_ITIMERVAL
-struct itimerval {
-	struct timeval it_interval;
-	struct timeval it_value;
-};
-#endif
-
-#ifdef NO_SETITIMER
-static inline int git_setitimer(int which UNUSED,
-				const struct itimerval *value UNUSED,
-				struct itimerval *newvalue UNUSED) {
-	return 0; /* pretend success */
-}
-#undef setitimer
-#define setitimer(which,value,ovalue) git_setitimer(which,value,ovalue)
-#endif
-
-#ifndef NO_LIBGEN_H
-#include <libgen.h>
-#else
-#define basename gitbasename
-char *gitbasename(char *);
-#define dirname gitdirname
-char *gitdirname(char *);
-#endif
-
-#ifndef NO_ICONV
-#include <iconv.h>
-#endif
-
 #ifndef NO_OPENSSL
 #ifdef __APPLE__
 #undef __AVAILABILITY_MACROS_USES_AVAILABILITY
@@ -441,34 +224,6 @@ char *gitdirname(char *);
 # include <sys/sysinfo.h>
 #endif
 
-/* On most systems <netdb.h> would have given us this, but
- * not on some systems (e.g. z/OS).
- */
-#ifndef NI_MAXHOST
-#define NI_MAXHOST 1025
-#endif
-
-#ifndef NI_MAXSERV
-#define NI_MAXSERV 32
-#endif
-
-/* On most systems <limits.h> would have given us this, but
- * not on some systems (e.g. GNU/Hurd).
- */
-#ifndef PATH_MAX
-#define PATH_MAX 4096
-#endif
-
-#ifndef NAME_MAX
-#define NAME_MAX 255
-#endif
-
-typedef uintmax_t timestamp_t;
-#define PRItime PRIuMAX
-#define parse_timestamp strtoumax
-#define TIME_MAX UINTMAX_MAX
-#define TIME_MIN 0
-
 #ifndef PATH_SEP
 #define PATH_SEP ':'
 #endif
@@ -492,11 +247,6 @@ static inline int noop_core_config(const char *var UNUSED,
 #define platform_core_config noop_core_config
 #endif
 
-int lstat_cache_aware_rmdir(const char *path);
-#if !defined(__MINGW32__) && !defined(_MSC_VER)
-#define rmdir lstat_cache_aware_rmdir
-#endif
-
 #ifndef has_dos_drive_prefix
 static inline int git_has_dos_drive_prefix(const char *path UNUSED)
 {
@@ -824,25 +574,6 @@ static inline bool strip_suffix(const char *str, const char *suffix,
 	memcpy(_swap_b_ptr, _swap_buffer, sizeof(a));		\
 } while (0)
 
-#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
-
-#ifndef PROT_READ
-#define PROT_READ 1
-#define PROT_WRITE 2
-#define MAP_PRIVATE 1
-#endif
-
-#define mmap git_mmap
-#define munmap git_munmap
-void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
-int git_munmap(void *start, size_t length);
-
-#else /* NO_MMAP || USE_WIN32_MMAP */
-
-#include <sys/mman.h>
-
-#endif /* NO_MMAP || USE_WIN32_MMAP */
-
 #ifdef NO_MMAP
 
 /* This value must be multiple of (pagesize * 2) */
@@ -858,177 +589,15 @@ int git_munmap(void *start, size_t length);
 
 #endif /* NO_MMAP */
 
-#ifndef MAP_FAILED
-#define MAP_FAILED ((void *)-1)
-#endif
-
 #ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
 #define on_disk_bytes(st) ((st).st_size)
 #else
 #define on_disk_bytes(st) ((st).st_blocks * 512)
 #endif
 
-#ifdef NEEDS_MODE_TRANSLATION
-#undef S_IFMT
-#undef S_IFREG
-#undef S_IFDIR
-#undef S_IFLNK
-#undef S_IFBLK
-#undef S_IFCHR
-#undef S_IFIFO
-#undef S_IFSOCK
-#define S_IFMT   0170000
-#define S_IFREG  0100000
-#define S_IFDIR  0040000
-#define S_IFLNK  0120000
-#define S_IFBLK  0060000
-#define S_IFCHR  0020000
-#define S_IFIFO  0010000
-#define S_IFSOCK 0140000
-#ifdef stat
-#undef stat
-#endif
-#define stat(path, buf) git_stat(path, buf)
-int git_stat(const char *, struct stat *);
-#ifdef fstat
-#undef fstat
-#endif
-#define fstat(fd, buf) git_fstat(fd, buf)
-int git_fstat(int, struct stat *);
-#ifdef lstat
-#undef lstat
-#endif
-#define lstat(path, buf) git_lstat(path, buf)
-int git_lstat(const char *, struct stat *);
-#endif
-
 #define DEFAULT_PACKED_GIT_LIMIT \
 	((1024L * 1024L) * (size_t)(sizeof(void*) >= 8 ? (32 * 1024L * 1024L) : 256))
 
-#ifdef NO_PREAD
-#define pread git_pread
-ssize_t git_pread(int fd, void *buf, size_t count, off_t offset);
-#endif
-
-#ifdef NO_SETENV
-#define setenv gitsetenv
-int gitsetenv(const char *, const char *, int);
-#endif
-
-#ifdef NO_MKDTEMP
-#define mkdtemp gitmkdtemp
-char *gitmkdtemp(char *);
-#endif
-
-#ifdef NO_UNSETENV
-#define unsetenv gitunsetenv
-int gitunsetenv(const char *);
-#endif
-
-#ifdef NO_STRCASESTR
-#define strcasestr gitstrcasestr
-char *gitstrcasestr(const char *haystack, const char *needle);
-#endif
-
-#ifdef NO_STRLCPY
-#define strlcpy gitstrlcpy
-size_t gitstrlcpy(char *, const char *, size_t);
-#endif
-
-#ifdef NO_STRTOUMAX
-#define strtoumax gitstrtoumax
-uintmax_t gitstrtoumax(const char *, char **, int);
-#define strtoimax gitstrtoimax
-intmax_t gitstrtoimax(const char *, char **, int);
-#endif
-
-#ifdef NO_HSTRERROR
-#define hstrerror githstrerror
-const char *githstrerror(int herror);
-#endif
-
-#ifdef NO_MEMMEM
-#define memmem gitmemmem
-void *gitmemmem(const void *haystack, size_t haystacklen,
-		const void *needle, size_t needlelen);
-#endif
-
-#ifdef OVERRIDE_STRDUP
-#ifdef strdup
-#undef strdup
-#endif
-#define strdup gitstrdup
-char *gitstrdup(const char *s);
-#endif
-
-#ifdef NO_GETPAGESIZE
-#define getpagesize() sysconf(_SC_PAGESIZE)
-#endif
-
-#ifndef O_CLOEXEC
-#define O_CLOEXEC 0
-#endif
-
-#ifdef FREAD_READS_DIRECTORIES
-# if !defined(SUPPRESS_FOPEN_REDEFINITION)
-#  ifdef fopen
-#   undef fopen
-#  endif
-#  define fopen(a,b) git_fopen(a,b)
-# endif
-FILE *git_fopen(const char*, const char*);
-#endif
-
-#ifdef SNPRINTF_RETURNS_BOGUS
-#ifdef snprintf
-#undef snprintf
-#endif
-#define snprintf git_snprintf
-int git_snprintf(char *str, size_t maxsize,
-		 const char *format, ...);
-#ifdef vsnprintf
-#undef vsnprintf
-#endif
-#define vsnprintf git_vsnprintf
-int git_vsnprintf(char *str, size_t maxsize,
-		  const char *format, va_list ap);
-#endif
-
-#ifdef OPEN_RETURNS_EINTR
-#undef open
-#define open git_open_with_retry
-int git_open_with_retry(const char *path, int flag, ...);
-#endif
-
-#ifdef __GLIBC_PREREQ
-#if __GLIBC_PREREQ(2, 1)
-#define HAVE_STRCHRNUL
-#endif
-#endif
-
-#ifndef HAVE_STRCHRNUL
-#define strchrnul gitstrchrnul
-static inline char *gitstrchrnul(const char *s, int c)
-{
-	while (*s && *s != c)
-		s++;
-	return (char *)s;
-}
-#endif
-
-#ifdef NO_INET_PTON
-int inet_pton(int af, const char *src, void *dst);
-#endif
-
-#ifdef NO_INET_NTOP
-const char *inet_ntop(int af, const void *src, char *dst, size_t size);
-#endif
-
-#ifdef NO_PTHREADS
-#define atexit git_atexit
-int git_atexit(void (*handler)(void));
-#endif
-
 static inline size_t st_add(size_t a, size_t b)
 {
 	if (unsigned_add_overflows(a, b))
@@ -1295,12 +864,6 @@ static inline size_t xsize_t(off_t len)
 	return (size_t) len;
 }
 
-#ifndef HOST_NAME_MAX
-#define HOST_NAME_MAX 256
-#endif
-
-#include "sane-ctype.h"
-
 /*
  * Like skip_prefix, but compare case-insensitively. Note that the comparison
  * is done via tolower(), so it is strictly ASCII (no multi-byte characters or
@@ -1366,34 +929,6 @@ static inline int strtol_i(char const *s, int base, int *result)
 	return 0;
 }
 
-void git_stable_qsort(void *base, size_t nmemb, size_t size,
-		      int(*compar)(const void *, const void *));
-#ifdef INTERNAL_QSORT
-#define qsort git_stable_qsort
-#endif
-
-#define QSORT(base, n, compar) sane_qsort((base), (n), sizeof(*(base)), compar)
-static inline void sane_qsort(void *base, size_t nmemb, size_t size,
-			      int(*compar)(const void *, const void *))
-{
-	if (nmemb > 1)
-		qsort(base, nmemb, size, compar);
-}
-
-#define STABLE_QSORT(base, n, compar) \
-	git_stable_qsort((base), (n), sizeof(*(base)), compar)
-
-#ifndef HAVE_ISO_QSORT_S
-int git_qsort_s(void *base, size_t nmemb, size_t size,
-		int (*compar)(const void *, const void *, void *), void *ctx);
-#define qsort_s git_qsort_s
-#endif
-
-#define QSORT_S(base, n, compar, ctx) do {			\
-	if (qsort_s((base), (n), sizeof(*(base)), compar, ctx))	\
-		BUG("qsort_s() failed");			\
-} while (0)
-
 #ifndef REG_STARTEND
 #error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
 #endif
@@ -1418,39 +953,12 @@ int git_regcomp(regex_t *preg, const char *pattern, int cflags);
 # define FORCE_DIR_SET_GID 0
 #endif
 
-#ifdef NO_NSEC
-#undef USE_NSEC
-#define ST_CTIME_NSEC(st) 0
-#define ST_MTIME_NSEC(st) 0
-#else
-#ifdef USE_ST_TIMESPEC
-#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))
-#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))
-#else
-#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
-#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
-#endif
-#endif
-
 #ifdef UNRELIABLE_FSTAT
 #define fstat_is_reliable() 0
 #else
 #define fstat_is_reliable() 1
 #endif
 
-#ifndef va_copy
-/*
- * Since an obvious implementation of va_list would be to make it a
- * pointer into the stack frame, a simple assignment will work on
- * many systems.  But let's try to be more portable.
- */
-#ifdef __va_copy
-#define va_copy(dst, src) __va_copy(dst, src)
-#else
-#define va_copy(dst, src) ((dst) = (src))
-#endif
-#endif
-
 /* usage.c: only to be used for testing BUG() implementation (see test-tool) */
 extern int BUG_exit_code;
 
@@ -1480,41 +988,6 @@ void bug_fl(const char *file, int line, const char *fmt, ...);
 # define SHELL_PATH "/bin/sh"
 #endif
 
-#ifndef _POSIX_THREAD_SAFE_FUNCTIONS
-static inline void git_flockfile(FILE *fh UNUSED)
-{
-	; /* nothing */
-}
-static inline void git_funlockfile(FILE *fh UNUSED)
-{
-	; /* nothing */
-}
-#undef flockfile
-#undef funlockfile
-#undef getc_unlocked
-#define flockfile(fh) git_flockfile(fh)
-#define funlockfile(fh) git_funlockfile(fh)
-#define getc_unlocked(fh) getc(fh)
-#endif
-
-#ifdef FILENO_IS_A_MACRO
-int git_fileno(FILE *stream);
-# ifndef COMPAT_CODE_FILENO
-#  undef fileno
-#  define fileno(p) git_fileno(p)
-# endif
-#endif
-
-#ifdef NEED_ACCESS_ROOT_HANDLER
-int git_access(const char *path, int mode);
-# ifndef COMPAT_CODE_ACCESS
-#  ifdef access
-#  undef access
-#  endif
-#  define access(path, mode) git_access(path, mode)
-# endif
-#endif
-
 /*
  * Our code often opens a path to an optional file, to work on its
  * contents when we can successfully open it.  We can ignore a failure
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 17/18] reftable: decouple from Git codebase by pulling in "compat/posix.h"

The reftable library includes "git-compat-util.h" in order to get a POSIX-like programming environment that papers over various differences between platforms. The header also brings with it a couple of helpers specific to the Git codebase though, and over time we have started to use these helpers in the reftable library, as well.

This makes it very hard to use the reftable library as a standalone library without the rest of the Git codebase, so other libraries like e.g. libgit2 cannot easily use it. But now that we have removed all calls to Git-specific functionality and have split out "compat/posix.h" as a separate header we can address this.

Stop including "git-compat-util.h" and instead include "compat/posix.h" to finalize the decoupling of the reftable library from the rest of the Git codebase. The only bits which remain specific to Git are "system.h" and "system.c", which projects will have to provide.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/system.c | 2 ++
 reftable/system.h | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
Show changes to 2 files +3 −1

reftable/system.c, reftable/system.h

diff --git a/reftable/system.c b/reftable/system.c
index e25ccc0da3c..1ee268b125d 100644
--- a/reftable/system.c
+++ b/reftable/system.c
@@ -1,3 +1,5 @@
+#include "../git-compat-util.h"
+
 #include "system.h"
 #include "basics.h"
 #include "reftable-error.h"
diff --git a/reftable/system.h b/reftable/system.h
index bb6a7e6285b..10055fbff2d 100644
--- a/reftable/system.h
+++ b/reftable/system.h
@@ -11,7 +11,7 @@ license that can be found in the LICENSE file or at
 
 /* This header glues the reftable library to the rest of Git */
 
-#include "git-compat-util.h"
+#include "compat/posix.h"
 #include "compat/zlib-compat.h"
 
 /*
-- 
2.48.1.666.gff9fcf71b7.dirty
Patrick Steinhardt· Feb 18, 2025, 09:20 UTC · re: Patrick Steinhardt · lore

[PATCH v6 18/18] Makefile: skip reftable library for Coccinelle

The reftable library does not use any of the common helpers that the Git project has. Consequently, most of the rules that we have in Coccinelle do not apply to the library at all and may even generate false positives when a pattern can be converted to use a Git helper function.

Exclude reftable library sources from being checked by Coccinelle to avoid such false positives.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Makefile +1 −1
diff --git a/Makefile b/Makefile
index cd0806e1e69..83f1df5502b 100644
--- a/Makefile
+++ b/Makefile
@@ -955,7 +955,7 @@ FOUND_SOURCE_FILES := $(filter-out $(GENERATED_H),$(shell $(SOURCES_CMD)))
 FOUND_C_SOURCES = $(filter %.c,$(FOUND_SOURCE_FILES))
 FOUND_H_SOURCES = $(filter %.h,$(FOUND_SOURCE_FILES))
 
-COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES),$(FOUND_C_SOURCES))
+COCCI_SOURCES = $(filter-out $(THIRD_PARTY_SOURCES) reftable/%,$(FOUND_C_SOURCES))
 
 LIB_H = $(FOUND_H_SOURCES)
 
-- 
2.48.1.666.gff9fcf71b7.dirty
Junio C Hamano· Feb 18, 2025, 18:55 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v6 00/18] reftable: stop using "git-compat-util.h"

Patrick Steinhardt <ps@pks.im> writes:
Show 13 quoted lines
> Changes in v6:
>   - I have rebased the patch series on top of "master" at 03944513488
>     (The eleventh batch, 2025-02-14). On the one hand this gets rid of
>     the conflict with the zlib-ng changes, which have meanwhile been
>     merged to that branch. And on the other hand it pulls in the new CI
>     build for MSVC+Meson, which allows me to have better test coverage
>     on Windows to better detect breakage there.
>   - Stop moving around Win32 files so that we do the minimum viable
>     change in this context. Hopefully, this makes it easier for Dscho to
>     integrate into Git for Windows. If it doesn't I don't really have
>     any other good ideas for how to do it, so the only alternative would
>     be to just ignore Windows altogether.
>   - Link to v5: https://lore.kernel.org/r/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im

Thanks. I think the "unlink" thing on Windows I have was queued depended on the previous iteration of this series so let me make sure that I'd unentangle it while replacing the topic with this round.

Will queue.  Thanks.
Junio C Hamano· Mar 11, 2025, 23:29 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 00/18] reftable: stop using "git-compat-util.h"

Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
>
>> Changes in v6:
>>   - I have rebased the patch series on top of "master" at 03944513488
>>     (The eleventh batch, 2025-02-14). On the one hand this gets rid of
>>     the conflict with the zlib-ng changes, which have meanwhile been
>>     merged to that branch. And on the other hand it pulls in the new CI
>>     build for MSVC+Meson, which allows me to have better test coverage
>>     on Windows to better detect breakage there.
>>   - Stop moving around Win32 files so that we do the minimum viable
>>     change in this context. Hopefully, this makes it easier for Dscho to
>>     integrate into Git for Windows. If it doesn't I don't really have
>>     any other good ideas for how to do it, so the only alternative would
>>     be to just ignore Windows altogether.
>>   - Link to v5: https://lore.kernel.org/r/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im
>
> Thanks.  I think the "unlink" thing on Windows I have was queued
> depended on the previous iteration of this series so let me make
> sure that I'd unentangle it while replacing the topic with this
> round.
>
> Will queue.  Thanks.

So, sorry I lost track. I think I saw you ask Dscho about the Windows specific bits once in a thread on What's cooking report, but I do not recall the outcome. Is this topic (and the unlink fix that depends on this series) now OK, or would we see further updates once 2.49 is tagged?

Thanks.
Patrick Steinhardt· Mar 12, 2025, 06:51 UTC · re: Junio C Hamano · lore

Re: [PATCH v6 00/18] reftable: stop using "git-compat-util.h"

On Tue, Mar 11, 2025 at 04:29:27PM -0700, Junio C Hamano wrote:
Show 30 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Patrick Steinhardt <ps@pks.im> writes:
> >
> >> Changes in v6:
> >>   - I have rebased the patch series on top of "master" at 03944513488
> >>     (The eleventh batch, 2025-02-14). On the one hand this gets rid of
> >>     the conflict with the zlib-ng changes, which have meanwhile been
> >>     merged to that branch. And on the other hand it pulls in the new CI
> >>     build for MSVC+Meson, which allows me to have better test coverage
> >>     on Windows to better detect breakage there.
> >>   - Stop moving around Win32 files so that we do the minimum viable
> >>     change in this context. Hopefully, this makes it easier for Dscho to
> >>     integrate into Git for Windows. If it doesn't I don't really have
> >>     any other good ideas for how to do it, so the only alternative would
> >>     be to just ignore Windows altogether.
> >>   - Link to v5: https://lore.kernel.org/r/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im
> >
> > Thanks.  I think the "unlink" thing on Windows I have was queued
> > depended on the previous iteration of this series so let me make
> > sure that I'd unentangle it while replacing the topic with this
> > round.
> >
> > Will queue.  Thanks.
> 
> So, sorry I lost track.  I think I saw you ask Dscho about the
> Windows specific bits once in a thread on What's cooking report,
> but I do not recall the outcome.  Is this topic (and the unlink fix
> that depends on this series) now OK, or would we see further updates
> once 2.49 is tagged?

There was a class of test failures caused by mismatching allocators in Git for Windows for which I've sent a proposed fix for downstream [1]. I've been talking to Johannes on Monday and he mentioned that he'll likely end up taking that patch as the pragmatic fix. Which to the best of my knowledge would mean that this series here is unblocked and can start to move forward again. Johannes, please correct me in case I'm misstating anything here.

So based on that I don't plan to reroll and hope that we can move forward with the series once Git v2.49.0 is out.

Thanks!
Patrick
[1]: <Z8bQsflIqzZd3AMs@pks.im>
Patrick Steinhardt· Mar 20, 2025, 15:17 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v6 00/18] reftable: stop using "git-compat-util.h"

On Wed, Mar 12, 2025 at 07:51:41AM +0100, Patrick Steinhardt wrote:
Show 42 quoted lines
> On Tue, Mar 11, 2025 at 04:29:27PM -0700, Junio C Hamano wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> > 
> > > Patrick Steinhardt <ps@pks.im> writes:
> > >
> > >> Changes in v6:
> > >>   - I have rebased the patch series on top of "master" at 03944513488
> > >>     (The eleventh batch, 2025-02-14). On the one hand this gets rid of
> > >>     the conflict with the zlib-ng changes, which have meanwhile been
> > >>     merged to that branch. And on the other hand it pulls in the new CI
> > >>     build for MSVC+Meson, which allows me to have better test coverage
> > >>     on Windows to better detect breakage there.
> > >>   - Stop moving around Win32 files so that we do the minimum viable
> > >>     change in this context. Hopefully, this makes it easier for Dscho to
> > >>     integrate into Git for Windows. If it doesn't I don't really have
> > >>     any other good ideas for how to do it, so the only alternative would
> > >>     be to just ignore Windows altogether.
> > >>   - Link to v5: https://lore.kernel.org/r/20250207-pks-reftable-drop-git-compat-util-v5-0-ba2adc79110f@pks.im
> > >
> > > Thanks.  I think the "unlink" thing on Windows I have was queued
> > > depended on the previous iteration of this series so let me make
> > > sure that I'd unentangle it while replacing the topic with this
> > > round.
> > >
> > > Will queue.  Thanks.
> > 
> > So, sorry I lost track.  I think I saw you ask Dscho about the
> > Windows specific bits once in a thread on What's cooking report,
> > but I do not recall the outcome.  Is this topic (and the unlink fix
> > that depends on this series) now OK, or would we see further updates
> > once 2.49 is tagged?
> 
> There was a class of test failures caused by mismatching allocators in
> Git for Windows for which I've sent a proposed fix for downstream [1].
> I've been talking to Johannes on Monday and he mentioned that he'll
> likely end up taking that patch as the pragmatic fix. Which to the best
> of my knowledge would mean that this series here is unblocked and can
> start to move forward again. Johannes, please correct me in case I'm
> misstating anything here.
> 
> So based on that I don't plan to reroll and hope that we can move
> forward with the series once Git v2.49.0 is out.

Quick status update: Johannes mentioned in [1] that he is fine with moving forward with this topic now. Thanks!

Patrick
[1]: https://github.com/gitgitgadget/git/pull/1874#issuecomment-2740342672

← back to recent threads