8
\$\begingroup\$ 
long long combine(unsigned int high, unsigned int low) {
    return ((unsigned long long) high) << 32 || low;
}
  1. Is there a better name for the operation?
  2. Is the implicit cast from unsigned long long to long long a reinterpret_cast, or what happens if the unsigned number is too large for the signed data type?
Improve this question
\$\endgroup\$
2
  • 6
    \$\begingroup\$ Also, note that you should be using a bitwise or operator | rather than a logical or operator ||. \$\endgroup\$
    – Adam Davis
    May 25, 2011 at 18:15
  • \$\begingroup\$ Irk! Obviously. Thanks you for spotting it. :) \$\endgroup\$
    – Felix Dombek
    May 25, 2011 at 18:18

1 Answer 1

Reset to default
9
\$\begingroup\$

You should return unsigned long long and let the user decide what they want to do with the cast, especially if you want this to be generic.

I'd prefer a name such as u32tou64, uinttoull, or something more descriptive than combine. A lot of this will depend on your own naming standards, though.

Also, I'd consider being more pedantic:

return (((uint64_t) high) << 32) | ((uint64_t) low);

It's unlikely to make a difference because the code is essentially the same as yours, but it's easier to read and avoids extremely rare (but very troublesome to debug) casting issues. It may require a custom types header if your compiler doesn't support this type notation.

Also, consider making it a macro. There's little benefit to having it as a function - the operation itself will take far less time than the function call setup, call, and return, so the performance of a macro will be much higher. Further, it isn't going to take up much more program space than a function call, so there's little to gain by leaving it a real function.

Improve this answer
\$\endgroup\$
7
  • 1
    \$\begingroup\$ Also, it's quite possible that a union could be a more efficient way of doing this than a shift, although I would hope that most optimizing compilers make the distinction meaningless. \$\endgroup\$
    – Adam Davis
    May 25, 2011 at 17:51
  • 2
    \$\begingroup\$ Thanks. I refrained from using unions because MS discourages them for alignment reasons (msdn.microsoft.com/en-us/library/ms724284%28v=VS.85%29.aspx, "Remarks", 3rd paragraph). An optimizing compiler should also inline my function. But thanks for the suggestion, I'll probably make it a macro anyways. \$\endgroup\$
    – Felix Dombek
    May 25, 2011 at 18:05
  • 7
    \$\begingroup\$ Since this is C++, an inline function should be pretty much the same as a macro. \$\endgroup\$
    – Winston Ewert
    May 25, 2011 at 23:53
  • \$\begingroup\$ You should not be using UINT64, its so unportable. \$\endgroup\$
    – mathepic
    May 27, 2011 at 23:04
  • \$\begingroup\$ @mathepic I always use a typesdef.h file or similar anyway. What typenames do you suggest are more portable for those that don't? \$\endgroup\$
    – Adam Davis
    May 28, 2011 at 2:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.