Fix infinite loop in ns_turn_server.c

[Asmir]

In case ur_map_get returns 1 server will enter infinite loop because newid != 0.

[ggarber]

Thank you for looking at this @Asmir . Good catch! Should we better refactor that code in this way?

do {
        if (TURN_RANDOM_SIZE == sizeof(mobile_id_t)) {
          newid = (mobile_id_t)turn_random();
        } else {
          newid = (mobile_id_t)turn_random();
          newid = (newid << 32) + (mobile_id_t)turn_random();
        }
        newid = newid & 0x00FFFFFFFFFFFFFFLL;
        if (!newid) {
           continue
        }
        newid = newid | sid;
    } while (ur_map_get(map, (ur_map_key_type)newid, NULL));

It looks to me like the current approach is overcomplicated and it is what led to this kind of bug that you found. What do you think?

[Asmir]

This will not work because after if (!newId) check the continue statement will still cause ur_map_get to be called but with newid = 0, which is not what we want.

[ggarber]

This will not work because after if (!newId) check the continue statement will still cause ur_map_get to be called but with newid = 0, which is not what we want.

ok, but can we fix that with:

} while (newid ==0 || ur_map_get(map, (ur_map_key_type)newid, NULL));

I think we need to fix the current double nested iteration

[Asmir]

This will not work because after if (!newId) check the continue statement will still cause ur_map_get to be called but with newid = 0, which is not what we want.

ok, but can we fix that with:

} while (newid ==0 || ur_map_get(map, (ur_map_key_type)newid, NULL));

I think we need to fix the current double nested iteration

That would be a good solution.

[ggarber]

This will not work because after if (!newId) check the continue statement will still cause ur_map_get to be called but with newid = 0, which is not what we want.

ok, but can we fix that with:

} while (newid ==0 || ur_map_get(map, (ur_map_key_type)newid, NULL));

I think we need to fix the current double nested iteration

That would be a good solution.

Can you apply it and do some basic testing if you can? Or you want me to do it? Thank you very much @Asmir

[Asmir]

This will not work because after if (!newId) check the continue statement will still cause ur_map_get to be called but with newid = 0, which is not what we want.

ok, but can we fix that with:

} while (newid ==0 || ur_map_get(map, (ur_map_key_type)newid, NULL));

I think we need to fix the current double nested iteration

That would be a good solution.

Can you apply it and do some basic testing if you can? Or you want me to do it? Thank you very much @Asmir

If you don't mind applying the change since I moved to other projects and don't have my test environment.