-2

I get a mask as a string that I retrieve using strtol and strtok and that I want to save in an unsigned int x, trail with zeros, and & it with an IP represented also as unsigned int in order to keep the MSBs only (At the end I'm going to compare that IP with another).

My idea was to set all the bits of x, and then turn off all bits placed after the mask size:

#define IPV4_LENGTH 32

//subent is "123.34.45.66/31"

curr_IP->address = malloc(sizeof (struct in_addr));
token = strtok(subnet, "/");
inet_aton(token, curr_IP->address);
long mask  = strtol(strtok(NULL, "/"), NULL, 10);
curr_IP->x = -1;
for(long i=mask; i<=IPV4_LENGTH; i++){
    curr_IP->x &= ~(1U << i);
}

Example: if mask = 31, I want to end with 11111111111111111111111111111110 represented in unsigned int -> FFFFFFFE in HEX

P.S mask can't be 0

UPDATE: Memory view after curr_IP->x = -1;

We can see it indeed sets up all the bits

enter image description here

Memory view after curr_IP->x &= ~(1U << i); when i = 31

expected fffffffe

enter image description here

RedYoel
  • 302
  • 2
  • 16
  • 1
    Please try to create a proper [mre] to show us. Hard-code the "input", and include things like `IPV4_LENGTH`. – Some programmer dude Dec 20 '21 at 09:50
  • Also try to use a debugger to step through your code statement by statement while monitoring variables and their values. It helps if you split up more complex expressions into smaller and simpler expressions, assigned to temporary variables. For example, `curr_IP->x &= ~(1UL << i);` could be `unsigned long temp1 = 1UL << i; unsigned long temp2 = ~temp1; unsigned long temp3 = curr_IP->x & temp2; curr_IP->x = temp3;` Makes it easier to see and verify your expressions and calculations. – Some programmer dude Dec 20 '21 at 09:53
  • @Someprogrammerdude Hope this is enough – RedYoel Dec 20 '21 at 10:21
  • My take on this: `curr_IP->x &= ~(1UL << i);` when `i = 32`. That means `curr_IP->x &= ~(1UL << 32UL)` and `(1UL << 32UL)` is 0 -> `curr_IP->x &= ~(0);` -> `curr_IP->x &= 0xFFFFFFFF;` – Morten Jensen Dec 20 '21 at 11:09
  • @MortenJensen I've changed `i=mask+1` to `i=mask` and updated the MEM view. I've also realized that I should use `int` instead of `long` because I need 32 bits only. – RedYoel Dec 20 '21 at 12:37
  • 1
    @RedYoel Remember that the size of types depends on compiler. For example, the Microsoft compiler still uses 32-bit `long`, even on 64-bit system. – Some programmer dude Dec 20 '21 at 12:50
  • 3
    @RedYoel `int` is signed and signed overflow is undefined behavior! So use an `unsigned int` or include `` and use an `uint32_t` to get 32-bits nomatter the compiler – Morten Jensen Dec 20 '21 at 12:52
  • @Someprogrammerdude You're right, I'll take Morten Jensen advice and use `uint32_t` – RedYoel Dec 20 '21 at 13:19

1 Answers1

1

I choose a different approach; turning off the mask and then setting the relevant bits to 1.

long mask  = strtol(strtok(NULL, "/"), NULL, 10);
curr_IP->x = 0; //mask to 0
for(long i=IPV4_LENGTH - mask; i<IPV4_LENGTH; i++){
    curr_IP->x |= (1U << i); //Sets the ith bit of the mask
}

Edit: As Morten Jensen pointed, it is better not to mix signed and unsigned integers, so the corrected code looks like that:

unsigned long mask  = strtol(strtok(NULL, "/"), NULL, 10);
curr_IP->x = 0; //mask to 0
for(unsigned long i=IPV4_LENGTH - mask; i<IPV4_LENGTH; i++){
    curr_IP->x |= (1U << i); //Sets the ith bit of the mask
}

Also, I struggled with debugging the code myself because I used the Memory view without realizing that I should read it from right to left.

In picture #2, I thought that I got fffffff7 when it was actually 7fffffff

RedYoel
  • 302
  • 2
  • 16