3

I'm currently trying to implement a google authenticator for an open source server and they have this small bit of code

  if (securityFlags & 0x01)               // PIN input
                {
                    pkt << uint32(0);
                    pkt << uint64(0) << uint64(0);      // 16 bytes hash?
                    // This triggers the 2 factor authenticator entry to popup on the client side

                }

                if (securityFlags & 0x02)               // Matrix input
                {
                    pkt << uint8(0);
                    pkt << uint8(0);
                    pkt << uint8(0);
                    pkt << uint8(0);
                    pkt << uint64(0);
                }

                if (securityFlags & 0x04)               // Security token input
                {
                    pkt << uint8(1);
                }

I'm just trying to figure out why they use pkt << uint32(0), as they seem completely redundant to me. And they also use it a lot of times over, which makes even less sense.

Any ideas why their code was written like that?

Rakete1111
  • 47,013
  • 16
  • 123
  • 162
Datsik
  • 14,453
  • 14
  • 80
  • 121
  • I hope I understood your question correctly, if not, please roll back my edit. – Rakete1111 Jan 24 '17 at 19:33
  • The calls are helping the compiler to know which of the overloaded versions of the '<<' operator it should use. The literal '0' could be implicitly converted into a number of types including int and unsigned int. If you use an explicit conversion you can get the compiler to follow your lead and call the overload with the matching signature. This helps in the situation where the compiler doesn't know what to do and exits with an ambiguous function call compile error. – london-deveoper Jan 24 '17 at 19:36
  • See http://stackoverflow.com/questions/6233132/c-compiler-error-ambiguous-call-to-overloaded-function for more information – london-deveoper Jan 24 '17 at 19:40

2 Answers2

11

operator<< is overloaded for ByteBuffer (this is a pkt type), and it looks as follows:

https://github.com/mangostwo/server/blob/b8ce9508483375a36699c309bce36810c4548007/src/shared/ByteBuffer.h#L138

    ByteBuffer& operator<<(uint8 value)
    {
        append<uint8>(value);
        return *this;
    }

so its not a shift by 0, but appending of value 0.

marcinj
  • 48,511
  • 9
  • 79
  • 100
  • So it just keeps adding 0 to the end of the packet? So to know what values to append would you need to know what the client side packet is expecting to be in those places? – Datsik Jan 24 '17 at 19:39
  • @Datsik Yes, to know what values to append you would need to know the protocol. – simon Jan 24 '17 at 20:00
  • 2
    Wow, that looks like a really bad idea for operator overloading. When you are already in low-level code which performs bit operations, with bit-wise `&` operators, hex literals and fixed-width unsigned variables floating around, overloading a bit-shift operator to mean something other than bit-shifting is exactly what you need to make your code easier to understand :) – Christian Hackl Jan 24 '17 at 20:26
  • @ChristianHackl Maybe I misinterpret, but I'd rather blame the one who decided to mix high-level code with low-level code, and not the one who implemented `ByteBuffer`. I think the operator overloading is fine. If `append` is public he should have used it instead (`pkt.append(0);`). – simon Jan 24 '17 at 21:06
  • @gurka: Well, I don't know who is to be blamed, of course, but the end result is just not nice. At the very least, a comment such as `// NOT bit-shifting but calling overloaded operator<<` would be in order. – Christian Hackl Jan 24 '17 at 21:12
1

pkt is likely of a class-type which overloads operator<<; if it weren't so, all those statements would have no effect since it's result is not used. (pkt << 0 rather than pkt <<= 0).

edmz
  • 8,220
  • 2
  • 26
  • 45