Code review comment for lp://qastaging/~2591kuldeep/libdrizzle/libdrizzle-binlogapi

Some observations on the unmarshaling code in helper.cc:

+uint32_t getByte3(int pos,const unsigned char* data)
+{
+ if((int)(sizeof(data)-pos)<3)
+ {

I don't think this is doing what you want, since sizeof(data) will just return the size of a pointer on the machine, not the size of the data buffer pointed to by data. If you want to know that, you'll need to pass it in to the function somehow, e.g. as a "data_len" argument.

The gratuitous cast to (int) is sometimes a sign of badness. size_t is not the same size as int on all platforms, for example. What is the actual range of possible values for pos? IMHO it would be better to write this conditional so as not to need a cast, for example
   if ((pos+3) > size_of_data_buffer)

assuming that both pos and size_of_data_buffer are of integer types guaranteed to be large enough to hold the size of a data buffer.

+ return UINT_MAX;

Returning an in-range error value for this kind of error makes me uncomfortable. For getByte4(), etc, how does the caller know whether the call succeeded?

For these particular functions, the only way they can fail is if there isn't enough data in the buffer. Since that the caller will have to test for the error return and handle it, it makes just as much sense for the check to be the caller's responsibility. This ends up requiring no more code at the calling site (in fact, it can use less code, if you can combine several checks) and simplifies getByteN()'s API (since you don't have to figure out how to return an error indication that is distinguishable from a success indication).

+ tmpMask=((uint32_t)data[pos]&tmpMask);

There are some macros in <libdrizzle-5.1/constants.h> for doing this kind of unpacking; you may be able to simplify the code by using them.