Suppressed assertion about block_size on windows

Hi,

On windows, the value of the alignment parameter to xmallocaligned, seems to occationally be 1 in some runs of tkrzw_file_perf, where it's a copy of PositionalParallelFileImpl::block_size_, while it's value is assert-ed to be at least sizeof(void*) in tkrzw_lib_common.cc:59:

58      #elif defined(_SYS_WINDOWS_)
59 --->   assert(alignment >= sizeof(void*));
60        size = AlignNumber(size, alignment);
61        void* ptr = _aligned_malloc(size, alignment);
62        if (ptr == nullptr) {
63          throw std::bad_alloc();
64        }
65        return ptr;
66      #else

This is most likely a consequence of it's value having been default initialized to 1 in the constructor of PositionalParallelFileImpl as well as the value passed to its' Setter-Procedure, SetAccessStrategy, invoked from tkrzw_file_perf.cc:283, defaulting to 1 in tkrzw_file_perf.cc:276, too.

In VCMakefile:165, among others, tkrzw_file_perf is invoked without explicitly specififying the --block_size, therefore a defaut value of 1 is used, and the statement asserted in tkrzw_lib_common.cc:59 is not provided.

Though the assertion is in a Windows specific branch, all Windows builds suppress it's evaluation by inconditionally setting the NDEBUG macro in VCMakefile:35.

As the assertion in tkrzw_lib_common.cc:59 never has an effect due to the macro, it shouldn't be there. Also, assert should not be used to handle invalid input the implemenation can recover from, and this error seems to be recoverable, since it's currently taking place.

Thanks.

Having block-size=1 is not harmful because it doesn't call xmallocaligned. https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_sys_file_pos_windows.h#L541

As for the unconditional setting of /DNDEBUG in VCMakefile, I'd like to make it conditional. Having a dynamic mechanism like ./configure on Windows is ideal but I don't know how. Anyway, let me keep the assertion in C++ for a while.

On Sat, Oct 26, 2024 at 1:36 AM Alonso Schaich @.***> wrote:

Hi,

On windows, the block_size of tkrzw_file_perf seems to be 1 when it's value is assert-ed to be at least sizeof(void*) in tkrzw_lib_common.cc:59 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_lib_common.cc#L59

This is most likely a consequence of it's value having been default initialized to 1 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_sys_file_pos_windows.h#L83 in the constructor of PositionalParallelFileImpl as well as the value passed to its' Setter-Procedure, SetAccessStrategy https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_sys_file_pos_windows.h#L428, invoked from tkrzw_file_perf.cc:283 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_file_perf.cc#L283, defaulting to 1 in tkrzw_file_perf.cc https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_file_perf.cc#L276, too.

In VCMakefile:165 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/VCMakefile#L165, among others, tkrzw_file_perf is invoked without explicitly specififying the block-size value, therefore a defaut value of 1 is used, and the statement asserted in tkrzw_lib_common.cc:59 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_lib_common.cc#L59 is not provided.

Though the assertion is in a Windows specific branch, all Windows builds suppress it's evaluation by inconditionally setting the NDEBUG macro in VCMakefile:35 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/VCMakefile#L35 .

As the assertion in tkrzw_lib_common.cc:59 https://github.com/estraier/tkrzw/blob/ba89aa1af261cbf22189ac15729ef6a7f4040fb5/tkrzw_lib_common.cc#L59 never has an effect due to the macro, it shouldn't be there.

— Reply to this email directly, view it on GitHub https://github.com/estraier/tkrzw/issues/57, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQGJVRBRB4JPHE66OWOGFITZ5JXSPAVCNFSM6AAAAABQTUK3GGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGYYTINJZGM4DOMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

estraier / tkrzw

Suppressed assertion about block_size on windows #57