Skip to content

Commit 6f3b0c0

Browse files
dgpvapoelstra
authored andcommitted
Improve comments for surctionproof init+alloc/destroy funcs
The comments with 'XXX' was intended to indicate that the listed concerns was subject to review and change, but the code with these comments was merged straight away. This commit replaces comments with more complete text describing the issues. This also signifies that the commit that this code was introduced in is not anymore 'work in progress'.
1 parent 250ebb3 commit 6f3b0c0

File tree

1 file changed

+13
-2
lines changed

1 file changed

+13
-2
lines changed

src/modules/surjection/main_impl.h

+13-2
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,10 @@ static size_t secp256k1_surjectionproof_csprng_next(secp256k1_surjectionproof_cs
151151
}
152152
}
153153

154-
/* XXX secp256k1_surjectionproof_create is not a good name, because it can be confused with secp256k1_surjectionproof_generate */
154+
/* While '_allocate_initialized' may be a wordy suffix for this function, and '_create'
155+
* may have been more appropriate, '_create' could be confused with '_generate',
156+
* as the meanings for the words are close. Therefore, more wordy, but less
157+
* ambiguous suffix was chosen. */
155158
int secp256k1_surjectionproof_allocate_initialized(const secp256k1_context* ctx, secp256k1_surjectionproof** proof_out_p, size_t *input_index, const secp256k1_fixed_asset_tag* fixed_input_tags, const size_t n_input_tags, const size_t n_input_tags_to_use, const secp256k1_fixed_asset_tag* fixed_output_tag, const size_t n_max_iterations, const unsigned char *random_seed32) {
156159
int ret = 0;
157160
secp256k1_surjectionproof* proof;
@@ -174,7 +177,15 @@ int secp256k1_surjectionproof_allocate_initialized(const secp256k1_context* ctx,
174177
return ret;
175178
}
176179

177-
/* XXX add checks to prevent destroy of stack-allocated struct ? */
180+
/* secp256k1_surjectionproof structure may also be allocated on the stack,
181+
* and initialized explicitly via secp256k1_surjectionproof_initialize().
182+
* Supplying stack-allocated struct to _destroy() will result in calling
183+
* free() with the pointer that points at the stack, with disasterous
184+
* consequences. Thus, it is not advised to mix heap- and stack-allocating
185+
* approaches to working with this struct. It is possible to detect this
186+
* situation by using additional field in the struct that can be set to
187+
* special value depending on the allocation path, and check it here.
188+
* But currently, it is not seen as big enough concern to warrant this extra code .*/
178189
void secp256k1_surjectionproof_destroy(secp256k1_surjectionproof* proof) {
179190
if (proof != NULL) {
180191
VERIFY_CHECK(proof->n_inputs <= SECP256K1_SURJECTIONPROOF_MAX_N_INPUTS);

0 commit comments

Comments
 (0)