Re: [PATCH] hash: fix pointer alignment

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Feb 26, 2026 at 04:43:26PM +0000, Radu Nicolau wrote:
> 
> On 26-Feb-26 4:22 PM, Marat Khalili wrote:
> > > diff --git a/lib/hash/rte_hash_crc.h b/lib/hash/rte_hash_crc.h
> > > index fa07c97685..66f11fafcd 100644
> > > --- a/lib/hash/rte_hash_crc.h
> > > +++ b/lib/hash/rte_hash_crc.h
> > > @@ -127,6 +127,24 @@ rte_hash_crc(const void *data, uint32_t data_len, uint32_t init_val)
> > >   	unsigned i;
> > >   	uintptr_t pd = (uintptr_t) data;
> > > 
> > > +	/* align input to 8 byte boundary if needed */
> > > +	if ((pd & 0x7) && data_len >= 8) {
> > Perhaps the case data_len < 8 should also be included, with each of the if's below checking and correcting data_len individually?
> 
> No need to include this case; if data_len < 8 it will skip the for loop and
> fall through those 3 if's, and get processed there.
> 
> 
> > 
> > > +		uintptr_t unaligned_bytes = 8 - (pd & 0x7);
> > > +		data_len -= unaligned_bytes;
> > > +		if (unaligned_bytes & 0x4) {
> > > +			init_val = rte_hash_crc_4byte(*(const uint32_t *)pd, init_val);
> > > +			pd += 4;
> > > +		}
> > > +		if (unaligned_bytes & 0x2) {
> > > +			init_val = rte_hash_crc_2byte(*(const uint16_t *)pd, init_val);
> > > +			pd += 2;
> > > +		}
> > > +		if (unaligned_bytes & 0x1) {
> > > +			init_val = rte_hash_crc_1byte(*(const uint8_t *)pd, init_val);
> > > +			pd += 1;
> > > +		}
> > Shouldn't the order be the opposite?
> As long as we process the right number of bytes the order doesn't matter.

But if we reverse the order we get more natural alignment. If the data
pointer is off-by-one, e.g. 0x65, and unaligned_bytes == 7, if we do as
here, we calculate the 4-byte version and 2-byte versions with 1-byte
alignment of the data. By reversing the order, we would do the 2-byte
calculation on 2-byte aligned data, and the 4-byte calc on 4-byte aligned
data etc.