common: pmem2_badblock_next is replaced by pmem2_badblock_next_internal #6133

grom72 · 2025-01-17T13:02:36Z

pmem2_badblock_next is no longer used as libpmem uses pmem2_badblock_next_internal.

This change is

osalyk

Reviewed 1 of 1 files at r1, 6 of 9 files at r3, 1 of 1 files at r4, 2 of 2 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @janekmi)

janekmi

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @grom72)

utils/style_check.sh line 52 at r4 (raw file):

	if [ $(echo "$clang_version 14.0" | tr " " "\n" | sort --version-sort | head -n 1) = $clang_version ]; then
		MSG="requires clang-format version >= 14.0 (version $clang_version installed)"

All of these is unnecessary. The issue is we started to use a wrong clang-format binary which happens to be version 18.1.3. But the version we want to use is still available. Please see the "Install dependencies" step. What we need to use is clang-format-14.

IMHO the patch should looks like this:

diff --git a/src/common.inc b/src/common.inc
index 94ac7fac3..a0cef4cdf 100644
--- a/src/common.inc
+++ b/src/common.inc
@@ -39,8 +39,8 @@ $(error Cannot evaluate version)
 endif
 
 ifeq ($(CLANG_FORMAT),)
-ifeq ($(shell command -v clang-format-9 > /dev/null && echo y || echo n), y)
-export CLANG_FORMAT ?= clang-format-9
+ifeq ($(shell command -v clang-format-14 > /dev/null && echo y || echo n), y)
+export CLANG_FORMAT ?= clang-format-14
 else
 export CLANG_FORMAT ?= clang-format
 endif
diff --git a/utils/style_check.sh b/utils/style_check.sh
index 34f5688fd..804c66f31 100755
--- a/utils/style_check.sh
+++ b/utils/style_check.sh
@@ -15,8 +15,8 @@ CHECK_TYPE=$1
 # When updating, please search for all references to "clang-format" and update
 # them as well; at this time these are CONTRIBUTING.md src/common.inc and
 # docker images.
-[ -z "$clang_format_bin" ] && which clang-format-9 >/dev/null &&
-	clang_format_bin=clang-format-9
+[ -z "$clang_format_bin" ] && which clang-format-14 >/dev/null &&
+	clang_format_bin=clang-format-14
 [ -z "$clang_format_bin" ] && which clang-format >/dev/null &&
 	clang_format_bin=clang-format
 [ -z "$clang_format_bin" ] && clang_format_bin=clang-format

Two remarks:

We have to stick to a certain version to explicitly avoid chasing the everchanging format requirements.
So, version ">= 14" never will be a thing. We can upgrade to version "==18" or any other else when the one we are using right now won't be available anymore.

janekmi

Reviewed 9 of 9 files at r3.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @grom72)

a discussion (no related file):

I do not understand how your solution to clang's build issues works. Please elaborate on that.
I am aware of two possible solutions:

2a. Just demote the cast-function-type-strict error back to a warning globally for clang.

diff --git a/src/Makefile.inc b/src/Makefile.inc
index b450609df..afa0cfbcd 100644
--- a/src/Makefile.inc
+++ b/src/Makefile.inc
@@ -35,6 +35,10 @@ DEFAULT_CFLAGS += -Wsign-compare
 DEFAULT_CFLAGS += -Wunused-parameter
 DEFAULT_CFLAGS += -fstack-usage
 
+ifeq ($(CC),clang)
+DEFAULT_CFLAGS += -Wno-error=cast-function-type-strict
+endif
+
 ifeq ($(WCONVERSION_AVAILABLE), y)
 DEFAULT_CFLAGS += -Wconversion
 endif
diff --git a/src/test/Makefile.inc b/src/test/Makefile.inc
index dc07375cb..bfd59f40e 100644
--- a/src/test/Makefile.inc
+++ b/src/test/Makefile.inc
@@ -406,6 +406,10 @@ CFLAGS += $(COMMON_FLAGS)
 
 CFLAGS += -Wsign-conversion
 
+ifeq ($(CC),clang)
+CFLAGS += -Wno-error=cast-function-type-strict
+endif
+
 ifeq ($(WUNREACHABLE_CODE_RETURN_AVAILABLE), y)
 CFLAGS += -Wunreachable-code-return
 endif

Ref: https://clang.llvm.org/docs/UsersManual.html#options-to-control-error-and-warning-messages

2b. Or ignore it case by case as you proposed but:

for clang (please see 1.) and
possibly using a define instead copy-paste this ugliness and
since gcc does not understand these #pragma you would also have to silence its unknown-pragmas error basically the same as 2a.

#define XXX_WRAP(exp) \
	_Pragma("clang diagnostic push") \
	_Pragma("clang diagnostic ignored \"-Wcast-function-type-strict\"") \
	exp; \
	_Pragma("clang diagnostic pop") \

/*
 * pmem_log_set_function -- set the log function pointer either to
 * a user-provided function pointer or to the default logging function.
 */
int
pmem_log_set_function(pmem_log_function *log_function)
{
	int ret = 0;
	XXX_WRAP(ret = core_log_set_function((core_log_function *)log_function));
	return core_log_error_translate(ret);
}

DEFAULT_CFLAGS += -Wno-error=unknown-pragmas

src/test/ctl_prefault/ctl_prefault.c line 123 at r3 (raw file):

	int open = atoi(argv[3]);

#pragma GCC diagnostic push

.

src/libpmem/libpmem.c line 133 at r3 (raw file):

{

#pragma GCC diagnostic push

Why GCC? I did not see a problem with gcc builds other than these three:

All of these the same:

libpmem.c: In function ‘pmem_log_set_function’:
libpmem.c:132: error: ignoring ‘#pragma clang diagnostic’ [-Werror=unknown-pragmas]
  132 | #pragma clang diagnostic ignored "-Wcast-function-type"
      | 
libpmem.c:134: error: ignoring ‘#pragma clang diagnostic’ [-Werror=unknown-pragmas]
  134 | #pragma clang diagnostic pop
      | 
cc1: all warnings being treated as errors

No indication of the cast-function-type warning/error for this compiler.

src/test/obj_memops/obj_memops.c line 165 at r3 (raw file):

test_redo(PMEMobjpool *pop, struct test_object *object)
{
#pragma GCC diagnostic push

.

src/test/obj_memops/obj_memops.c line 627 at r3 (raw file):

{
#define ULOG_SIZE 1024
#pragma GCC diagnostic push

.

src/test/obj_memops/obj_memops.c line 658 at r3 (raw file):

test_undo(PMEMobjpool *pop, struct test_object *object)
{
#pragma GCC diagnostic push

.

src/libpmemobj/obj_log.c line 44 at r3 (raw file):

pmemobj_log_set_function(pmemobj_log_function *log_function)
{
#pragma GCC diagnostic push

.

src/Makefile.inc line 41 at r3 (raw file):

ifeq ($(CC),clang)
DEFAULT_CFLAGS += -Wcast-function-type-strict
endif

This line looks odd. The warning (promoted to an error) is already there. Can you tell me how it works?

src/libpmemobj/lane.c line 219 at r3 (raw file):

		goto error_internal_new;

#pragma GCC diagnostic push

.

janekmi

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @grom72)

grom72

Reviewed 1 of 9 files at r3, 1 of 2 files at r7, 1 of 3 files at r8, 1 of 3 files at r10.
Reviewable status: 7 of 12 files reviewed, 10 unresolved discussions (waiting on @janekmi and @osalyk)

src/libpmem/libpmem.c line 133 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Why GCC? I did not see a problem with gcc builds other than these three:

https://github.com/pmem/pmdk/actions/runs/12830139068/job/35777639398

https://github.com/pmem/pmdk/actions/runs/12830165101/job/35777722027

https://github.com/pmem/pmdk/actions/runs/12830210588/job/35777866034

All of these the same:
libpmem.c: In function ‘pmem_log_set_function’:
libpmem.c:132: error: ignoring ‘#pragma clang diagnostic’ [-Werror=unknown-pragmas]
  132 | #pragma clang diagnostic ignored "-Wcast-function-type"
      | 
libpmem.c:134: error: ignoring ‘#pragma clang diagnostic’ [-Werror=unknown-pragmas]
  134 | #pragma clang diagnostic pop
      | 
cc1: all warnings being treated as errors
No indication of the cast-function-type warning/error for this compiler.

GCC ignores this pragma, but #pragma clang causes an error on GCC:
https://github.com/pmem/pmdk/actions/runs/12830165101/job/35777722027#step:5:271
we can add \-Wno-error=unknown-pragmas but it does not add any value and makes us blind for other unknown #pragmas

utils/style_check.sh line 52 at r4 (raw file):

Previously, janekmi (Jan Michalski) wrote…

All of these is unnecessary. The issue is we started to use a wrong clang-format binary which happens to be version 18.1.3. But the version we want to use is still available. Please see the "Install dependencies" step. What we need to use is clang-format-14.

IMHO the patch should looks like this:
diff --git a/src/common.inc b/src/common.inc
index 94ac7fac3..a0cef4cdf 100644
--- a/src/common.inc
+++ b/src/common.inc
@@ -39,8 +39,8 @@ $(error Cannot evaluate version)
 endif
 
 ifeq ($(CLANG_FORMAT),)
-ifeq ($(shell command -v clang-format-9 > /dev/null && echo y || echo n), y)
-export CLANG_FORMAT ?= clang-format-9
+ifeq ($(shell command -v clang-format-14 > /dev/null && echo y || echo n), y)
+export CLANG_FORMAT ?= clang-format-14
 else
 export CLANG_FORMAT ?= clang-format
 endif
diff --git a/utils/style_check.sh b/utils/style_check.sh
index 34f5688fd..804c66f31 100755
--- a/utils/style_check.sh
+++ b/utils/style_check.sh
@@ -15,8 +15,8 @@ CHECK_TYPE=$1
 # When updating, please search for all references to "clang-format" and update
 # them as well; at this time these are CONTRIBUTING.md src/common.inc and
 # docker images.
-[ -z "$clang_format_bin" ] && which clang-format-9 >/dev/null &&
-	clang_format_bin=clang-format-9
+[ -z "$clang_format_bin" ] && which clang-format-14 >/dev/null &&
+	clang_format_bin=clang-format-14
 [ -z "$clang_format_bin" ] && which clang-format >/dev/null &&
 	clang_format_bin=clang-format
 [ -z "$clang_format_bin" ] && clang_format_bin=clang-format
Two remarks:

We have to stick to a certain version to explicitly avoid chasing the everchanging format requirements.

So, version ">= 14" never will be a thing. We can upgrade to version "==18" or any other else when the one we are using right now won't be available anymore.

version clang-format-14 is not

src/Makefile.inc line 41 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This line looks odd. The warning (promoted to an error) is already there. Can you tell me how it works?

Done.

grom72

Reviewable status: 7 of 12 files reviewed, 10 unresolved discussions (waiting on @janekmi and @osalyk)

utils/style_check.sh line 52 at r4 (raw file):

Previously, grom72 (Tomasz Gromadzki) wrote…

version clang-format-14 is not

DONE

grom72

Reviewable status: 7 of 12 files reviewed, 10 unresolved discussions (waiting on @janekmi and @osalyk)

a discussion (no related file):

Previously, janekmi (Jan Michalski) wrote…

I do not understand how your solution to clang's build issues works. Please elaborate on that.
I am aware of two possible solutions:

2a. Just demote the cast-function-type-strict error back to a warning globally for clang.

diff --git a/src/Makefile.inc b/src/Makefile.inc
index b450609df..afa0cfbcd 100644
--- a/src/Makefile.inc
+++ b/src/Makefile.inc
@@ -35,6 +35,10 @@ DEFAULT_CFLAGS += -Wsign-compare
 DEFAULT_CFLAGS += -Wunused-parameter
 DEFAULT_CFLAGS += -fstack-usage
 
+ifeq ($(CC),clang)
+DEFAULT_CFLAGS += -Wno-error=cast-function-type-strict
+endif
+
 ifeq ($(WCONVERSION_AVAILABLE), y)
 DEFAULT_CFLAGS += -Wconversion
 endif
diff --git a/src/test/Makefile.inc b/src/test/Makefile.inc
index dc07375cb..bfd59f40e 100644
--- a/src/test/Makefile.inc
+++ b/src/test/Makefile.inc
@@ -406,6 +406,10 @@ CFLAGS += $(COMMON_FLAGS)
 
 CFLAGS += -Wsign-conversion
 
+ifeq ($(CC),clang)
+CFLAGS += -Wno-error=cast-function-type-strict
+endif
+
 ifeq ($(WUNREACHABLE_CODE_RETURN_AVAILABLE), y)
 CFLAGS += -Wunreachable-code-return
 endif

Ref: https://clang.llvm.org/docs/UsersManual.html#options-to-control-error-and-warning-messages

2b. Or ignore it case by case as you proposed but:

for clang (please see 1.) and
possibly using a define instead copy-paste this ugliness and
since gcc does not understand these #pragma you would also have to silence its unknown-pragmas error basically the same as 2a.

#define XXX_WRAP(exp) \
	_Pragma("clang diagnostic push") \
	_Pragma("clang diagnostic ignored \"-Wcast-function-type-strict\"") \
	exp; \
	_Pragma("clang diagnostic pop") \

/*
 * pmem_log_set_function -- set the log function pointer either to
 * a user-provided function pointer or to the default logging function.
 */
int
pmem_log_set_function(pmem_log_function *log_function)
{
	int ret = 0;
	XXX_WRAP(ret = core_log_set_function((core_log_function *)log_function));
	return core_log_error_translate(ret);
}

DEFAULT_CFLAGS += -Wno-error=unknown-pragmas

I have updated clang-format to ver. 14
I keep GCC pragma instead of clang pragma
I do not want to make any additional wrapper.

grom72

Reviewable status: 7 of 12 files reviewed, 10 unresolved discussions (waiting on @janekmi and @osalyk)

src/libpmemobj/lane.c line 219 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

.

src/libpmemobj/obj_log.c line 44 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

.

src/test/ctl_prefault/ctl_prefault.c line 123 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

.

src/test/obj_memops/obj_memops.c line 165 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

.

src/test/obj_memops/obj_memops.c line 627 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

.

src/test/obj_memops/obj_memops.c line 658 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

.

grom72

Reviewed 1 of 1 files at r1.
Reviewable status: 7 of 12 files reviewed, 10 unresolved discussions (waiting on @janekmi and @osalyk)

osalyk

Reviewed 3 of 6 files at r9, 3 of 3 files at r10, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @janekmi)

grom72

Reviewed 1 of 9 files at r3, 5 of 6 files at r9, 2 of 3 files at r10.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @janekmi)

janekmi

Reviewed 3 of 6 files at r9, 3 of 3 files at r10, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @grom72)

grom72

Reviewed 7 of 14 files at r13, 2 of 3 files at r14, 5 of 6 files at r16.
Reviewable status: 8 of 15 files reviewed, 8 unresolved discussions (waiting on @janekmi)

grom72

Reviewed 1 of 3 files at r14, 6 of 11 files at r15, 1 of 6 files at r16, 6 of 11 files at r17, 1 of 1 files at r18, all commit messages.
Reviewable status: 10 of 15 files reviewed, 8 unresolved discussions (waiting on @janekmi)

osalyk

Reviewed 3 of 11 files at r17, all commit messages.
Reviewable status: 13 of 15 files reviewed, 8 unresolved discussions (waiting on @janekmi)

janekmi

Reviewed 3 of 3 files at r14, 10 of 11 files at r17, 1 of 1 files at r18, all commit messages.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @grom72)

-- commits line 30 at r18:
Please squash these two.

Code quote:

New commits in r15 on 29/01/2025 at 11:26, probably rebased from r9:
- cb474ec: common: fix clang '-Wcast-function-type-strict' issue

  Signed-off-by: Tomasz Gromadzki <[email protected]>

New commits in r18 on 29/01/2025 at 12:06:
- 36fffdf: core: fix: cast-function-type -> ..-strict clang

  Signed-off-by: Tomasz Gromadzki <[email protected]>

src/libpmem/libpmem.c line 134 at r18 (raw file):

	int ret;
	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		ret = core_log_set_function((core_log_function *)log_function));

Redundant.

Suggestion:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE(
		ret = core_log_set_function((core_log_function *)log_function));

src/test/ctl_prefault/ctl_prefault.c line 126 at r18 (raw file):

		prefault_fun(prefault, \
			(fun)pmemobj_ctl_get, \
			(fun)pmemobj_ctl_set));

.

Code quote:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		prefault_fun(prefault, \
			(fun)pmemobj_ctl_get, \
			(fun)pmemobj_ctl_set));

src/test/ctl_prefault/ctl_prefault.c line 126 at r18 (raw file):

		prefault_fun(prefault, \
			(fun)pmemobj_ctl_get, \
			(fun)pmemobj_ctl_set));

Why you decided to break this line so differently?

Suggestion:

		prefault_fun(prefault, (fun)pmemobj_ctl_get, (fun)pmemobj_ctl_set));

src/libpmemobj/lane.c line 233 at r18 (raw file):

			LANE_UNDO_SIZE, \
			lane_undo_extend, (ulog_free_fn)pfree, &pop->p_ops, \
			LOG_TYPE_UNDO));

Redundant. Please apply everywhere.

Suggestion:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE(
		lane->external = operation_new(
			(struct ulog *)&layout->external,
			LANE_REDO_EXTERNAL_SIZE,
			lane_redo_extend, (ulog_free_fn)pfree, &pop->p_ops,
			LOG_TYPE_REDO));
	if (lane->external == NULL)
		goto error_external_new;

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE(
		lane->undo = operation_new(
			(struct ulog *)&layout->undo,
			LANE_UNDO_SIZE,
			lane_undo_extend, (ulog_free_fn)pfree, &pop->p_ops,
			LOG_TYPE_UNDO));

src/libpmemobj/obj_log.c line 3 at r18 (raw file):

// SPDX-License-Identifier: BSD-3-Clause
/* Copyright 2024, Intel Corporation */
/* Copyright 2025 Hewlett Packard Enterprise Development LP */

Why no comma?

Suggestion:

/* Copyright 2025, Hewlett Packard Enterprise Development LP */

src/libpmemobj/obj_log.c line 44 at r18 (raw file):

pmemobj_log_set_function(pmemobj_log_function *log_function)
{
	int ret;

I think it would be wise to 0/NULL initialize all these variables. Otherwise we may end up with passing down garbage. It is better to end up with SIGSEGV. Please apply everywhere.

src/libpmemobj/obj_log.c line 46 at r18 (raw file):

	int ret;
	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		ret = core_log_set_function((core_log_function *)log_function));

.

Code quote:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		ret = core_log_set_function((core_log_function *)log_function));

src/test/obj_memops/obj_memops.c line 170 at r18 (raw file):

			TEST_ENTRIES, pmalloc_redo_extend, \
			(ulog_free_fn)pfree, &pop->p_ops, \
		LOG_TYPE_REDO));

.

Code quote:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		ctx = operation_new((struct ulog *)&object->redo, \
			TEST_ENTRIES, pmalloc_redo_extend, \
			(ulog_free_fn)pfree, &pop->p_ops, \
		LOG_TYPE_REDO));

src/test/obj_memops/obj_memops.c line 170 at r18 (raw file):

			TEST_ENTRIES, pmalloc_redo_extend, \
			(ulog_free_fn)pfree, &pop->p_ops, \
		LOG_TYPE_REDO));

One indentation level too little.

Code quote:

		LOG_TYPE_REDO));

src/test/obj_memops/obj_memops.c line 630 at r18 (raw file):

		ctx = operation_new((struct ulog *)&object->redo, \
		TEST_ENTRIES, pmalloc_redo_extend, (ulog_free_fn)pfree, \
		&pop->p_ops, LOG_TYPE_REDO));

.

Code quote:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		ctx = operation_new((struct ulog *)&object->redo, \
		TEST_ENTRIES, pmalloc_redo_extend, (ulog_free_fn)pfree, \
		&pop->p_ops, LOG_TYPE_REDO));

src/test/obj_memops/obj_memops.c line 659 at r18 (raw file):

		ctx = operation_new((struct ulog *)&object->undo, \
			TEST_ENTRIES, pmalloc_redo_extend, \
			(ulog_free_fn)pfree, &pop->p_ops, LOG_TYPE_UNDO));

.

Code quote:

	CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE( \
		ctx = operation_new((struct ulog *)&object->undo, \
			TEST_ENTRIES, pmalloc_redo_extend, \
			(ulog_free_fn)pfree, &pop->p_ops, LOG_TYPE_UNDO));

src/core/util.h line 335 at r18 (raw file):

#define SUPPRESS_ARG_9(X, ...) SUPPRESS_ARG_1(X); SUPPRESS_ARG_8(__VA_ARGS__)

/* macro to ignore "cast-function-type-strict" warning in clang */

Please do not start a description of a macro with "macro...". It is obviously a macro.

Suggestion:

tell clang to ignore the "cast-function-type-strict" warning

src/core/util.h line 337 at r18 (raw file):

/* macro to ignore "cast-function-type-strict" warning in clang */
#if __clang__
#define CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE(exp) \

The "noun-verb-noun" order makes it a little bit easier to read and comprehend.

Suggestion:

CLANG_IGNORE_CAST_FUNCTION_TYPE_STRICT_WARNING

src/core/util.h line 341 at r18 (raw file):

_Pragma("clang diagnostic ignored \"-Wcast-function-type-strict\"") \
exp; \
_Pragma("clang diagnostic pop")

Indentation would be nice.

Suggestion:

#define CLANG_CAST_FUNCTION_TYPE_STRICT_WARNING_IGNORE(exp) \
	_Pragma("clang diagnostic push") \
	_Pragma("clang diagnostic ignored \"-Wcast-function-type-strict\"") \
	exp; \
	_Pragma("clang diagnostic pop")

grom72

Reviewable status: 3 of 15 files reviewed, 15 unresolved discussions (waiting on @janekmi and @osalyk)

src/core/util.h line 335 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Please do not start a description of a macro with "macro...". It is obviously a macro.

Done.

src/core/util.h line 337 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

The "noun-verb-noun" order makes it a little bit easier to read and comprehend.

Done.

src/core/util.h line 341 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Indentation would be nice.

Done.

src/libpmem/libpmem.c line 134 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Redundant.

Done.

src/libpmemobj/lane.c line 233 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Redundant. Please apply everywhere.

Done.

src/libpmemobj/obj_log.c line 3 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Why no comma?

Done.

src/libpmemobj/obj_log.c line 44 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I think it would be wise to 0/NULL initialize all these variables. Otherwise we may end up with passing down garbage. It is better to end up with SIGSEGV. Please apply everywhere.

Done.

src/libpmemobj/obj_log.c line 46 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

src/test/ctl_prefault/ctl_prefault.c line 126 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

src/test/ctl_prefault/ctl_prefault.c line 126 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Why you decided to break this line so differently?

Done.

src/test/obj_memops/obj_memops.c line 170 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

One indentation level too little.

Done.

src/test/obj_memops/obj_memops.c line 170 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

src/test/obj_memops/obj_memops.c line 630 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

src/test/obj_memops/obj_memops.c line 659 at r18 (raw file):

Previously, janekmi (Jan Michalski) wrote…

.

Done.

pmem2_badblock_next is no longer used as libpmem uses pmem2_badblock_next_internal. Signed-off-by: Tomasz Gromadzki <[email protected]>

grom72

Reviewed 9 of 9 files at r22.
Reviewable status: 1 of 15 files reviewed, 15 unresolved discussions (waiting on @janekmi and @osalyk)

-- commits line 30 at r18:

Previously, janekmi (Jan Michalski) wrote…

Please squash these two.

Done.

grom72

Reviewed 1 of 8 files at r23, 7 of 7 files at r24, all commit messages.
Reviewable status: 9 of 15 files reviewed, 15 unresolved discussions (waiting on @janekmi and @osalyk)

grom72

Reviewed 1 of 8 files at r23.
Reviewable status: 10 of 15 files reviewed, 15 unresolved discussions (waiting on @janekmi and @osalyk)

janekmi

Reviewed 7 of 8 files at r23, 6 of 7 files at r24, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72)

src/core/util.h line 335 at r24 (raw file):

#define SUPPRESS_ARG_9(X, ...) SUPPRESS_ARG_1(X); SUPPRESS_ARG_8(__VA_ARGS__)

/* tell clang to ignore "cast-function-type-strict" warning */

Suggestion:

tell clang to ignore the "cast-function-type-strict" warning

src/common.inc line 3 at r24 (raw file):

# SPDX-License-Identifier: BSD-3-Clause
# Copyright 2014-2023, Intel Corporation
# Copyright 2025, Hewlett Packard Enterprise Development LP

This comma should be a part of the "common: fix - clang-format update from 9.0 to 14.0" commit.

Code quote:

# Copyright 2025, Hewlett Packard Enterprise Development LP

utils/style_check.sh line 3 at r24 (raw file):

#!/usr/bin/env bash
# SPDX-License-Identifier: BSD-3-Clause
# Copyright 2016-2023, Intel Corporation

Missing copyright.

janekmi

Reviewed 1 of 7 files at r24.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @grom72)

grom72

Reviewable status: 3 of 15 files reviewed, 3 unresolved discussions (waiting on @janekmi)

src/common.inc line 3 at r24 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This comma should be a part of the "common: fix - clang-format update from 9.0 to 14.0" commit.

Done.

utils/style_check.sh line 3 at r24 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Missing copyright.

Done.

src/core/util.h line 335 at r24 (raw file):

#define SUPPRESS_ARG_9(X, ...) SUPPRESS_ARG_1(X); SUPPRESS_ARG_8(__VA_ARGS__)

/* tell clang to ignore "cast-function-type-strict" warning */

Done.

Signed-off-by: Tomasz Gromadzki <[email protected]>

janekmi

Reviewed 3 of 3 files at r28, 11 of 11 files at r29, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72)

osalyk

Reviewed 3 of 3 files at r28, 11 of 11 files at r29, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @grom72)

grom72 requested review from janekmi and osalyk January 17, 2025 13:02

grom72 added the no changelog Add to skip the changelog check on your pull request label Jan 17, 2025

grom72 force-pushed the 6126-fix-call-stack-analysis branch 11 times, most recently from 2467c68 to b747be4 Compare January 17, 2025 22:08

grom72 mentioned this pull request Jan 20, 2025

Regular main execution #6135

Merged

1 task

grom72 added the sprint goal This pull request is part of the ongoing sprint label Jan 21, 2025

grom72 mentioned this pull request Jan 21, 2025

common: ChangeLog supplement for #6117 and #6127 #6136

Merged

1 task

osalyk approved these changes Jan 21, 2025

View reviewed changes

grom72 added this to the 2.1.1 milestone Jan 23, 2025

janekmi requested changes Jan 28, 2025

View reviewed changes

janekmi reviewed Jan 28, 2025

View reviewed changes

grom72 force-pushed the 6126-fix-call-stack-analysis branch from 08ab880 to 6880aa1 Compare January 29, 2025 07:56

grom72 commented Jan 29, 2025

View reviewed changes

osalyk approved these changes Jan 29, 2025

View reviewed changes

grom72 commented Jan 29, 2025

View reviewed changes

janekmi reviewed Jan 29, 2025

View reviewed changes

grom72 force-pushed the 6126-fix-call-stack-analysis branch 5 times, most recently from 4061c9e to cf2701b Compare January 29, 2025 11:56

grom72 commented Jan 29, 2025

View reviewed changes

grom72 force-pushed the 6126-fix-call-stack-analysis branch 2 times, most recently from 200f6d6 to 36fffdf Compare January 29, 2025 12:06

grom72 commented Jan 29, 2025

View reviewed changes

osalyk approved these changes Jan 29, 2025

View reviewed changes

janekmi requested changes Jan 29, 2025

View reviewed changes

grom72 commented Jan 29, 2025

View reviewed changes

common: pmem2_badblock_next is replaced by pmem2_badblock_next_internal

4306bce

pmem2_badblock_next is no longer used as libpmem uses pmem2_badblock_next_internal. Signed-off-by: Tomasz Gromadzki <[email protected]>

grom72 force-pushed the 6126-fix-call-stack-analysis branch from 77db020 to c02799f Compare January 29, 2025 14:29

grom72 commented Jan 29, 2025

View reviewed changes

janekmi requested changes Jan 29, 2025

View reviewed changes

janekmi reviewed Jan 29, 2025

View reviewed changes

grom72 force-pushed the 6126-fix-call-stack-analysis branch from c02799f to 5e9dbc3 Compare January 29, 2025 14:48

grom72 commented Jan 29, 2025

View reviewed changes

grom72 added 2 commits January 29, 2025 15:52

common: fix - clang-format update from 9.0 to 14.0

d2a1988

Signed-off-by: Tomasz Gromadzki <[email protected]>

common: fix clang '-Wcast-function-type-strict' issue

7152ecf

Signed-off-by: Tomasz Gromadzki <[email protected]>

grom72 force-pushed the 6126-fix-call-stack-analysis branch from 5e9dbc3 to 7152ecf Compare January 29, 2025 14:52

janekmi approved these changes Jan 29, 2025

View reviewed changes

janekmi requested a review from osalyk January 29, 2025 19:23

osalyk approved these changes Jan 30, 2025

View reviewed changes

janekmi merged commit e62b373 into pmem:master Jan 30, 2025
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

common: pmem2_badblock_next is replaced by pmem2_badblock_next_internal #6133

common: pmem2_badblock_next is replaced by pmem2_badblock_next_internal #6133

grom72 commented Jan 17, 2025 •

edited

Loading

osalyk left a comment

janekmi left a comment •

edited

Loading

janekmi left a comment

janekmi left a comment

grom72 left a comment

grom72 left a comment

grom72 left a comment

grom72 left a comment

grom72 left a comment

osalyk left a comment

grom72 left a comment

janekmi left a comment

grom72 left a comment

grom72 left a comment

osalyk left a comment

janekmi left a comment

grom72 left a comment

grom72 left a comment

grom72 left a comment

grom72 left a comment

janekmi left a comment

janekmi left a comment

grom72 left a comment

janekmi left a comment

osalyk left a comment

common: pmem2_badblock_next is replaced by pmem2_badblock_next_internal #6133

common: pmem2_badblock_next is replaced by pmem2_badblock_next_internal #6133

Conversation

grom72 commented Jan 17, 2025 • edited Loading

osalyk left a comment

Choose a reason for hiding this comment

janekmi left a comment • edited Loading

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

osalyk left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

osalyk left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

grom72 left a comment

Choose a reason for hiding this comment

janekmi left a comment

Choose a reason for hiding this comment

osalyk left a comment

Choose a reason for hiding this comment

grom72 commented Jan 17, 2025 •

edited

Loading

janekmi left a comment •

edited

Loading