On Wed, Jul 02, 2025 at 02:14:15PM -0800, Junio C Hamano wrote:
> Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> writes:
>
> > - length = sizeof(int64_t);
> > - if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0))
> > + length = sizeof(physical_memory);
> > + if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
> > + if (length < sizeof(physical_memory)) {
> > + unsigned bits = (sizeof(physical_memory) - length) * 8;
> > +
> > + physical_memory <<= bits;
> > + physical_memory >>= bits;
>
> I do not quite understand this version. Does the correctness of
> this depend on the machine having a certain byte-order?
Yes, sorry and as you pointed out it is obviously incorrect and should had
been instead something like (barelly tested, though so please let me make
sure and not waste more of your time)
uint64_t physical_memory = 0;
...
if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
# if GIT_BYTE_ORDER == GIT_BIG_ENDIAN
if (length < sizeof(physical_memory)) {
unsigned bits = (sizeof(physical_memory) - length) * 8;
physical_memory >>= bits;
}
# endif
return physycal_nenory
}
> then shifting it down by 32-bits to the right may fill the upper half
> with 1 if the result in the 4-byte long is more than 2GB because
> the type of physical_memory is signed, and then we cast that value
> to u64. Which does not sound correct, either.
note that I changed the type to unsigned previously, but the rest was
obviously wrong.
the shifting was meant to be a cooler way to get those bits cleared,
because I thought that relying in the initialization wasn't as cool
from the previous comments.
> Would it make more sense to pass &u64 and return it only when
> length==8 as you did in v2 while removing the need to cast?
v2 (without the cast) is indeed enough and better, but my concern was
that this affects 32-bit FreeBSD which will be returning always 0.
a fixed version of this, would allow at least a better return, and
because most of the extra work is only needed in Big Endian (which
could only affect Power) then it is almost a free upgrade.
Carlo
