| Summary: | Implement missing features of SDL_vsnprintf, namely string width & precision | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Anthony @ POW Games <ant> |
| Component: | *don't know* | Assignee: | Ozkan Sezer <sezeroz> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | normal | ||
| Priority: | P2 | CC: | sezeroz |
| Version: | 2.0.8 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
patch for string.c
patch implementing precision for the integral value printers tests |
||
|
Description
Anthony @ POW Games
2018-09-20 01:57:08 UTC
Hey Ozkan, do you want to take a look at this? Will try looking (no promises or an ETA.) Created attachment 3312 [details]
patch for string.c
Here is an initial patch for string.c which does the following:
- make %.* correctly parse precision.
- make %* correctly parse width.
- make string printer honor the precision.
Minimally tested, so please review carefully and test carefully.
For the record, I tested the patch using the following minimal C code
on Linux:
#include <stdio.h>
#include "SDL.h"
char foo[] = "abcdefghi";
char bar[64];
int main (void) {
printf("%.*s\n",6,foo);
printf("%.6s\n",foo);
printf("%10.*s\n",6,foo);
printf("%*.6s\n",10,foo);
SDL_snprintf(bar,sizeof(bar),"%.*s",6,foo);
printf("%s\n",bar);
SDL_snprintf(bar,sizeof(bar),"%.6s",foo);
printf("%s\n",bar);
SDL_snprintf(bar,sizeof(bar),"%10.*s",6,foo);
printf("%s\n",bar);
SDL_snprintf(bar,sizeof(bar),"%*.6s",10,foo);
printf("%s\n",bar);
return 0;
}
Offhand it looks good. Could you also implement them for the numeric print formatters? Thanks! > Offhand it looks good. I pushed the existing patch in two parts with slight modification: https://hg.libsdl.org/SDL/rev/4cbf7e663cb2 https://hg.libsdl.org/SDL/rev/3c5adcfe014e > Could you also implement them for the numeric print formatters? OK, not closing this yet and will look. Created attachment 3316 [details] patch implementing precision for the integral value printers (In reply to Sam Lantinga from comment #5) > Could you also implement them for the numeric print > formatters? Attached patch implements precision for integral value printers. It might not be the most efficient, but it seems to do the job. It also _fixes_ it because, without this, SDL_PrintString() makes a mess now that it honors precision for strings, and the integral value printers do call SDL_PrintString(). Please review. Will look at float value printing later. This is missing check for maxlen, but otherwise looks good. Thanks! Applied with maxlen checking added: https://hg.libsdl.org/SDL/rev/d3b8ea488be8 Please review / test this final state: if all good, then let's close this. As for float prints, there already is precision handling in there AFAICS (looked only briefly), so I won't touch it unless something is actually wrong with it. Anthony: Had you a chance to test the latest code in hg? I don't know how to compile from source for Windows, but I managed to test the latest hg on Android and yes, all appears ok, in so far as "*" width for strings. I haven't done an extensive test of all the features though. Would you like me to try to break it to make sure it's solid before closing this? Thanks for pulling together to get this working - impressive. I should try to contribute code in the future, but I don't feel worthy :-| (In reply to Anthony @ POW Games from comment #11) > I don't know how to compile from source for Windows, Sam: can we point to a buildbot windows build for him? > but I managed to test > the latest hg on Android and yes, all appears ok, in so far as "*" width > for strings. Assuming Android is configured without HAVE_LIBC, then this is good. > I haven't done an extensive test of all the features though. Would you > like me to try to break it to make sure it's solid before closing this? No need, I guess, but would be interesting to see such a report. (Not every single feature of vsnprintf() is really implemented in there, so it shouldn't be hard to break.) > Thanks for pulling together to get this working You're welcome. We don't have the buildbot builds accessible right now, but I'm going to put up an RC build this weekend and he can check it out there. Anthony: there are 2.0.9 prerelease builds here, which include the SDL_vsnprintf() updates: http://www.libsdl.org/tmp/download-2.0.php You can use them to test on windows. That's awesome, thank you. I'll try them out now. I have some issues with the latest Android build not running on Android 4.2.2 (anything pre-Kitkat I suspect). And issues with the joysticks on Android being opened as keyboards and some keyboards being opened as joysticks (the HUD stuff seems to have made things worse). I'll open bugs repots when I know more. Do I need to close this if it's ok, or can anyone? (In reply to Anthony @ POW Games from comment #15) > Do I need to close this if it's ok, or can anyone? You can close it if things are OK. Just noticed the "+" flag isn't implemented for integers (this forces a + or - to be written, regardless). That's the only thing I've noticed so far for SDL_vsnprintf whiles testing 2.0.9 with my game. (In reply to Anthony @ POW Games from comment #17) > Just noticed the "+" flag isn't implemented for integers (this forces a + or > - to be written, regardless). That's the only thing I've noticed so far for > SDL_vsnprintf whiles testing 2.0.9 with my game. Sam: Do we want the '+' flag for 2.0.9, or leave it for later times? Include it in 2.0.9, please. Else I will need to use stdio's version - I can ditch stdio entirely if this works. I'm having a few issues with 2.0.9 on Android. Let me raise them before roll-out. Pushed a fix after the width / precision updates that had gone in: https://hg.libsdl.org/SDL/rev/2dbf011db466 Well, I went ahead and implemented '+' for signed integers printing: https://hg.libsdl.org/SDL/rev/f1ac9de30ee1 Can we close this now? Created attachment 3330 [details]
tests
FWIW, here are the tests I used myself when working with this.
I wonder if it's necessary to "info.force_sign = SDL_FALSE;" for unsigned? But otherwise looks good. Many thanks. |