Skip to content

Conversation

herumi
Copy link
Contributor

@herumi herumi commented Dec 10, 2017

I have changed the return value of blsXXXDeserialize() if MCLBN_USE_NEW_DESERIALIZE_API is defined as the followings.

#ifdef MCLBN_USE_NEW_DESERIALIZE_API
// return read byte size if success else 0
#else
// return 0 if success else -1
#endif
BLS_DLL_API mclRetType blsIdDeserialize(blsId *id, const void *buf, mclSize bufSize);
BLS_DLL_API mclRetType blsSecretKeyDeserialize(blsSecretKey *sec, const void *buf, mclSize bufSize);
BLS_DLL_API mclRetType blsPublicKeyDeserialize(blsPublicKey *pub, const void *buf, mclSize bufSize);
BLS_DLL_API mclRetType blsSignatureDeserialize(blsSignature *sig, const void *buf, mclSize bufSize);

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 99.298% when pulling 3d52efb on new_deserialize_api into f5c388f on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling c0a8e75 on new_deserialize_api into f5c388f on master.

}
}

function wrapDeserialize (func) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this replicates a bit of what is in wrapInput maybe we could use something like this instead

function wrapDeserialize (func) {
  func = wrapInput(func)
  return function () {
    const r = func.apply(null, arguments)
    if (r === 0) {
      throw new Error('Deserialize err')
    }
  }
}

what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. I fixed it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

awesome! looking good 👍

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 362393e on new_deserialize_api into f5c388f on master.

@wanderer wanderer merged commit 3c007f5 into master Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants