Skip to content
This repository was archived by the owner on Jul 27, 2022. It is now read-only.

C binding for lmctfy#13

Open
monnand wants to merge 84 commits into
google:masterfrom
monnand:cbinding
Open

C binding for lmctfy#13
monnand wants to merge 84 commits into
google:masterfrom
monnand:cbinding

Conversation

@monnand

@monnand monnand commented Feb 10, 2014

Copy link
Copy Markdown

C binding for lmctfy.

  • It requires protobuf-c to generate c code for .proto
  • All function names prefixed by lmctfy_
  • For any method which requires a protobuf generated class as its parameter or its return value, there will be two functions: one's name follows the original method name; another's name is suffixed by _raw. The _raw functions takes a serialized buffer to represent the protobuf generated object.
  • Any method which returns a ::util::Status or ::util::StatusOr has a corresponding function in the c library which takes a struct status * as its first parameter. The status structure will be used as output. Those functions will also check the error code in the status structure before executing other code in the functions. If the error code is not zero, they will return immediately with the error code.

Comment thread clmctfy/include/clmctfy.h Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move the includes into "/includes"? I think the c bindings implementation is still okay to have where it is.

@monnand

monnand commented Mar 12, 2014

Copy link
Copy Markdown
Author

I noticed that the parameter order does not follow the coding style (Should be input first then output). I will change them later.

@monnand

monnand commented Mar 13, 2014

Copy link
Copy Markdown
Author

Please take another look.

Besides those lines mentioned in the reviews, I made the following changes as well:

  • Rearranged the layout of files, so that every .cc file has its corresponding header file.
  • Changed the order of parameters of all functions.

@monnand

monnand commented Apr 19, 2014

Copy link
Copy Markdown
Author

I just made a rebase onto 0.4.5. Please take another look.

@rjnagal

rjnagal commented Apr 19, 2014

Copy link
Copy Markdown
Contributor

Thanks. Ignore the travis error - its not fully setup. There's a jenkins build and test that should be firing up.

@lmctfy-bot

Copy link
Copy Markdown

Can one of the admins verify this patch?

@rjnagal

rjnagal commented Apr 19, 2014

Copy link
Copy Markdown
Contributor

ok to test

@lmctfy-bot

Copy link
Copy Markdown

All tests passing.
Refer to this link for build results: http://23.251.156.144:8080/job/lmctfy-pull-build/17/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants