Skip to content
This repository was archived by the owner on Mar 12, 2020. It is now read-only.

Conversation

@howjmay
Copy link
Contributor

@howjmay howjmay commented Jan 13, 2020

<>
In transaction_serialize_to_flex_trits() the value of
macro NUM_TRITS_* and NUM_FLEX_TRITS_* may not
always be the same values, so we need to unify them.

Test Plan:

<<Outline how the reviewer can verify & test your changes here>>

In `transaction_serialize_to_flex_trits()` the value of
macro `NUM_TRITS_*` and `NUM_FLEX_TRITS_*` may not
always be the same values, so we need to unify them.
@thibault-martinez
Copy link
Contributor

Hi @howjmay, the documentation of this function says that the size should be in trits, not flex_trits.

/// Inserts the contents of an array into another array starting at a given
/// index.
/// @param[in] to_flex_trits - the array to insert into
/// @param[in] to_len - the number of trits encoded in the to_flex_trits array
/// @param[in] flex_trits - the array containing the trits to copy over
/// @param[in] len - the number of trits the flex_trits array stores
/// @param[in] start - the start index in the destination array
/// @param[in] num_trits - the number of trits to copy over
/// @return size_t - the number of trits copied over
size_t flex_trits_insert(flex_trit_t *const to_flex_trits, size_t const to_len, flex_trit_t const *const flex_trits,
                         size_t const len, size_t const start, size_t const num_trits);

@howjmay
Copy link
Contributor Author

howjmay commented Jan 13, 2020

However I wonder that if the sizes used here are in trits, then how could the size used in line 133 is NUM_FLEX_TRITS_SERIALIZED_TRANSACTION. Doesn't it cause bufferover flow?

@thibault-martinez
Copy link
Contributor

This line 133 https://github.com/iotaledger/entangled/pull/1472/files#diff-fcfdd8b4cc72f7c24ae51e4061656b3aR133 ?

It looks good to me, it's only flex_trits_insert that needs size in trits.

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.

2 participants