We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 4263 - Implement missing features of SDL_vsnprintf, namely string width & precision
Summary: Implement missing features of SDL_vsnprintf, namely string width & precision
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: *don't know* (show other bugs)
Version: 2.0.8
Hardware: All All
: P2 normal
Assignee: Ozkan Sezer
QA Contact: Sam Lantinga
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-09-20 01:57 UTC by Anthony @ POW Games
Modified: 2018-09-29 01:43 UTC (History)
1 user (show)

See Also:


Attachments
patch for string.c (2.19 KB, patch)
2018-09-25 20:53 UTC, Ozkan Sezer
Details | Diff
patch implementing precision for the integral value printers (1.78 KB, patch)
2018-09-26 17:09 UTC, Ozkan Sezer
Details | Diff
tests (2.69 KB, application/zip)
2018-09-28 22:43 UTC, Ozkan Sezer
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Anthony @ POW Games 2018-09-20 01:57:08 UTC
The precision and width of strings is ignored. There's todo in the source code that mentions this is missing. Can it be implemented pretty please? 

I'm trying to use "%.*s" where I specify a width using a parameter, but it's ignored. Even  "%.8s" doesn't work.
Comment 1 Sam Lantinga 2018-09-24 23:34:26 UTC
Hey Ozkan, do you want to take a look at this?
Comment 2 Ozkan Sezer 2018-09-25 06:18:21 UTC
Will try looking (no promises or an ETA.)
Comment 3 Ozkan Sezer 2018-09-25 20:53:32 UTC
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.
Comment 4 Ozkan Sezer 2018-09-25 20:58:45 UTC
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;
}
Comment 5 Sam Lantinga 2018-09-26 03:16:10 UTC
Offhand it looks good. Could you also implement them for the numeric print formatters?

Thanks!
Comment 6 Ozkan Sezer 2018-09-26 07:46:05 UTC
> 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.
Comment 7 Ozkan Sezer 2018-09-26 17:09:36 UTC
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.
Comment 8 Sam Lantinga 2018-09-26 21:02:24 UTC
This is missing check for maxlen, but otherwise looks good. Thanks!
Comment 9 Ozkan Sezer 2018-09-26 21:35:35 UTC
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.
Comment 10 Ozkan Sezer 2018-09-27 06:45:41 UTC
Anthony:  Had you a chance to test the latest code in hg?
Comment 11 Anthony @ POW Games 2018-09-27 18:21:31 UTC
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 :-|
Comment 12 Ozkan Sezer 2018-09-27 18:27:12 UTC
(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.
Comment 13 Sam Lantinga 2018-09-27 21:53:57 UTC
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.
Comment 14 Ozkan Sezer 2018-09-28 11:12:43 UTC
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.
Comment 15 Anthony @ POW Games 2018-09-28 12:56:35 UTC
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?
Comment 16 Ozkan Sezer 2018-09-28 14:06:21 UTC
(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.
Comment 17 Anthony @ POW Games 2018-09-28 18:08:06 UTC
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.
Comment 18 Ozkan Sezer 2018-09-28 18:36:25 UTC
(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?
Comment 19 Anthony @ POW Games 2018-09-28 18:49:34 UTC
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.
Comment 20 Ozkan Sezer 2018-09-28 21:51:36 UTC
Pushed a fix after the width / precision updates that had gone in:
https://hg.libsdl.org/SDL/rev/2dbf011db466
Comment 21 Ozkan Sezer 2018-09-28 22:25:54 UTC
Well, I went ahead and implemented '+' for signed integers printing:
https://hg.libsdl.org/SDL/rev/f1ac9de30ee1

Can we close this now?
Comment 22 Ozkan Sezer 2018-09-28 22:43:49 UTC
Created attachment 3330 [details]
tests

FWIW, here are the tests I used myself when working with this.
Comment 23 Anthony @ POW Games 2018-09-29 01:43:14 UTC
I wonder if it's necessary to "info.force_sign = SDL_FALSE;" for unsigned? But otherwise looks good. Many thanks.