Skip to content

Conversation

MuhammadSalah-MS
Copy link
Contributor

follows the bit manipulation style (p as MSB of the field) as demonstrated in K&R, particularly aligning with the getbits example (p+1-n). So bits to be set are the least after the position p.

follows the bit manipulation style (p as MSB of the field) as demonstrated in K&R, particularly aligning with the getbits example (p+1-n).
So bits to be set are the least after the  position p.
@ohkimur
Copy link
Owner

ohkimur commented Jun 26, 2025

Can you help me understand a bit better the context?

Copy link
Owner

@ohkimur ohkimur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello there, thanks for your contribution to this repository! Here are my thoughts abut the code after your proposed changes.

Suggested changes

Your PR fixes the shift offset but breaks the masking logic. The target field in x must be cleared before OR-ing bits from y; otherwise zeros from y won’t overwrite ones in x.

  • Shift should be p + 1 - n (or keep ++p and use p - n consistently).
  • Clear the n-bit field at position p in x before inserting y’s bits.

Proposed implementation

unsigned int setbits(int x, int p, int n, int y)
{
  unsigned int mask  = ~(~0u << n);         // n ones
  unsigned int shift = p + 1 - n;           // zero-based position

  unsigned int x_cleared = x & ~(mask << shift);
  unsigned int y_bits    = (y & mask) << shift;

  return x_cleared | y_bits;
}

If you prefer to keep ++p; in your version, use unsigned int shift = p - n; and keep the same clear/insert steps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants