Merge lp://qastaging/~2591kuldeep/libdrizzle/libdrizzle-binlogapi into lp://qastaging/libdrizzle

Proposed by kuldeep porwal
Status: Needs review
Proposed branch: lp://qastaging/~2591kuldeep/libdrizzle/libdrizzle-binlogapi
Merge into: lp://qastaging/libdrizzle
Diff against target: 3507 lines (+3312/-0)
35 files modified
docs/binlogapi/colum_value.rst (+37/-0)
docs/binlogapi/event_data.rst (+69/-0)
docs/binlogapi/event_header.rst (+26/-0)
docs/binlogapi/event_interface.rst (+10/-0)
docs/binlogapi/helper.rst (+209/-0)
docs/binlogapi/query_event.rst (+110/-0)
docs/binlogapi/row_event.rst (+196/-0)
docs/binlogapi/row_iterator.rst (+37/-0)
docs/binlogapi/table_map_event.rst (+130/-0)
docs/binlogapi/xid_event.rst (+75/-0)
docs/examples/binlog-read-api.rst (+110/-0)
libdrizzle-5.1/binlog_client.h (+15/-0)
libdrizzle-5.1/binlogapi.h (+36/-0)
libdrizzle-5.1/column_value.h (+39/-0)
libdrizzle-5.1/drizzle_client.h (+1/-0)
libdrizzle-5.1/event_data.h (+69/-0)
libdrizzle-5.1/event_header.h (+67/-0)
libdrizzle-5.1/event_interface.h (+51/-0)
libdrizzle-5.1/helper.h (+155/-0)
libdrizzle-5.1/include.am (+15/-0)
libdrizzle-5.1/query_event.h (+155/-0)
libdrizzle-5.1/row_event.h (+181/-0)
libdrizzle-5.1/row_iterator.h (+35/-0)
libdrizzle-5.1/table_map_event.h (+203/-0)
libdrizzle-5.1/xid_event.h (+95/-0)
libdrizzle/commonapi.h (+1/-0)
libdrizzle/event_data.cc (+61/-0)
libdrizzle/event_header.cc (+67/-0)
libdrizzle/helper.cc (+286/-0)
libdrizzle/include.am (+15/-0)
libdrizzle/query_event.cc (+126/-0)
libdrizzle/row_event.cc (+297/-0)
libdrizzle/row_iterator.cc (+16/-0)
libdrizzle/table_map_event.cc (+235/-0)
libdrizzle/xid_event.cc (+82/-0)
To merge this branch: bzr merge lp://qastaging/~2591kuldeep/libdrizzle/libdrizzle-binlogapi
Reviewer Review Type Date Requested Status
Wim Lewis (community) Needs Fixing
Stewart Smith (community) Approve
Review via email: mp+180569@code.qastaging.launchpad.net

Description of the change

Currently only have api to read header for MySQL binlog events. I have built the extended api to read event data also. (libdrizzle/binlogevent)

To post a comment you must log in.
Revision history for this message
Stewart Smith (stewart) wrote :

Approved with simple changes of adding copyright headers in all new source files (and modifying existing ones).

review: Approve
126. By kuldeep <kuldeep@kuldeep-laptop>

xid event added

127. By kuldeep <kuldeep@kuldeep-laptop>

copyright headers added

128. By kuldeep <kuldeep@kuldeep-laptop>

Bug fix for Row Event and modified api a bit for row_iterator

129. By kuldeep <kuldeep@kuldeep-laptop>

exception handling

130. By kuldeep <kuldeep@kuldeep-laptop>

Adding Tested code on local environment, now we can access the new api functions within library

131. By kuldeep <kuldeep@kuldeep-laptop>

Documentation Done, Resolved few dangling pointer bugs

132. By kuldeep <kuldeep@kuldeep-laptop>

Example added

Revision history for this message
Wim Lewis (wiml-omni) wrote :

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.

review: Needs Fixing

Unmerged revisions

132. By kuldeep <kuldeep@kuldeep-laptop>

Example added

131. By kuldeep <kuldeep@kuldeep-laptop>

Documentation Done, Resolved few dangling pointer bugs

130. By kuldeep <kuldeep@kuldeep-laptop>

Adding Tested code on local environment, now we can access the new api functions within library

129. By kuldeep <kuldeep@kuldeep-laptop>

exception handling

128. By kuldeep <kuldeep@kuldeep-laptop>

Bug fix for Row Event and modified api a bit for row_iterator

127. By kuldeep <kuldeep@kuldeep-laptop>

copyright headers added

126. By kuldeep <kuldeep@kuldeep-laptop>

xid event added

125. By kuldeep <kuldeep@kuldeep-laptop>

Added binlogevent api

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: