-
Notifications
You must be signed in to change notification settings - Fork 99
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
Public interface should provide safe buffer lengths #149
Comments
@expnkx Why not? Users currently have to copy and paste a magic number buried inside an internal source file. If the buffer size needs to change for some reason, those using the macro will automatically use the correct length upon recompilation. Unless something was added to the latest C standards, C does not provided a zero-cost numeric constant declaration other than |
I don't deny that, but there's no other zero-cost alternative in C (this library is written in C, and I want to use this library). With a reasonable prefix, the risk of pollution can be mitigated. "Don't hard-code arbitrary magic numbers" is a common coding standard. At the very least, the safe buffer size constant could be put in a separate header file that the user can optionally #include.
I'm not proposing to change the internal workings of this library. I'm just proposing that the magic number
Isn't dynamic allocation magnitudes slower than using a fixed-size buffer known at compile-time? It can also lead to heap fragmentation in small embedded environments. If I want to stringify thousands of floats in a loop using this library, reusing the same 25-byte buffer makes more sense than having this library dynamically allocate that buffer thousands of times. Of course I don't know the length of each stringified float, but there is a known maximum length and I want this library to provide me a compile-time symbol with the maximum possible length. Just so we're clear, my proposal (as well as my other recent ones), pertains to this library only. Not your implementation of Ryu of anybody else's. You can of course do whatever you want with your library. |
You are free to disagree on technical matters, but I will no longer tolerate any personal attacks from you. |
@ulfjack I have no way of contacting you privately, so I'm posting this here. You have a problem here with a user who keeps harassing the participants of your issue tracker with personal insults (which I have archived offline). That user is also actively discouraging the use of your library. I recommend that you take the appropriate measures, or your user base will end up turning away from this project: https://help.github.com/en/github/building-a-strong-community/blocking-a-user-from-your-personal-account |
@ulfjack I can provide you with screenshots of this issue thread before the user in question deleted the worst of the personal insults. If you have email notifications enabled, please review the original transcript of this thread. |
for c the choices are limited, maybe something like this:
then use the buffer directly or |
@jeaiii That's a clever way of avoiding macros while being clear about the intent. The only problem is that the string literal could possibly allocate a small amount of static memory. This is probably not a problem for most, but could be for some. How about this instead?
|
That would include padding bytes, but would be guaranteed to be sufficiently large. Obtaining the size of a struct member in C is a bit convoluted: https://stackoverflow.com/questions/3553296/sizeof-single-struct-member-in-c |
Yes, had to unlearn a lot to see what works in plain c! might go with this then, if concerned with strange padding scenarios,
|
I was just about to propose C array typedefs myself. To be clear, these type definitions would not be part of the ryu function signatures, but merely provided to allow the client to know (programmaticly) how large the buffers should be. |
I think you can specify padding of structs? I haven't done it in years. I personally would be fine with a macro, or even a comment in ryu.h ! Would love to see some documentation in the header file at the very least, so I know just from looking at the header file if my assumptions are correct. ie I had to read the code to check if d2exp() used malloc or an internal static buffer, etc. Would also be nice for the documentation to mention why d2s() has no precision option. Would be nice if there was a d2s(precision) option, the naive implementation would be to generate a fixed and an exp result at that precision and then return the shortest ... that would work, right? |
There should be
#define
macros in rhu.h which specifies safe buffer lengths for thebuffered
functions.The text was updated successfully, but these errors were encountered: