Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name public API structs so they can be forward declared #1627

Closed
achow101 opened this issue Oct 28, 2024 · 5 comments · Fixed by #1628
Closed

Name public API structs so they can be forward declared #1627

achow101 opened this issue Oct 28, 2024 · 5 comments · Fixed by #1628
Labels
Milestone

Comments

@achow101
Copy link
Member

Currently all of the structs in the public API are only typedef'd, rather than both named and typedef'd. As such, the structs cannot be forward declared and instead using the struct name (typedef) in a header requires including the library headers. This can be problematic for projects which want to reduce the inclusion tree as much as possible.


For some context behind this request, in my MuSig2 PR to Bitcoin Core, since the secp256k1_musig_secnonce cannot be copied or serialized, I have to pass around a pointer to the object in various functions, so many headers need to include secp256k1_musig.h just for the struct's declaration. Including secp256k1_musig.h in those headers result in needing to directly link libsecp256k1 in several of build targets that did not previously require it. This issue seems like it can be avoided by forward declaring that (and other) struct, but in order to do so, it needs to be named in the libsecp headers.

My current workaround is to pass void * and then cast them as necessary but I don't really like doing that.

@sipa
Copy link
Contributor

sipa commented Oct 29, 2024

Concept ACK, I guess. It shouldn't hurt us to turn every typedef struct { ... } secp256k1_foobar into typedef struct secp256k1_foobar_struct { ... } secp256k1_foobar?

@achow101
Copy link
Member Author

turn every typedef struct { ... } secp256k1_foobar into typedef struct secp256k1_foobar_struct { ... } secp256k1_foobar?

I'd prefer to have both names be the same, i.e. typedef struct secp256k1_foobar {...} secp256k1_foobar.

@jonasnick jonasnick added this to the 0.6.0 milestone Oct 30, 2024
@sipa
Copy link
Contributor

sipa commented Oct 31, 2024

@achow101 Want to open a PR?

@achow101
Copy link
Member Author

Sure

@achow101
Copy link
Member Author

#1628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants