Bug 73 - sr_si_string_u64() doesn't handle all cases
: sr_si_string_u64() doesn't handle all cases
Status: RESOLVED FIXED
Product: libsigrok
Classification: Unclassified
Component: Other
: unreleased development snapshot
: All All
: Normal normal
: libsigrok 0.2.1
Assigned To: Nobody
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-09 11:08 CET by Uwe Hermann
Modified: 2013-08-07 16:43 CEST (History)
2 users (show)



Attachments
Patch to format decimals correctly in sr_si_string_u64() (1.75 KB, patch)
2013-06-01 22:16 CEST, Peter Stuge
Details
This patch fixes missing leading zeroes, trims trailing, and makes sure tests are consistent re: trailing zeroes. (6.07 KB, text/plain)
2013-06-03 21:30 CEST, Paul Sokolovsky
Details
Fix sr_si_string_u64() test cases. (319 bytes, patch)
2013-06-04 02:41 CEST, Peter Stuge
Details
Format decimals correctly in sr_si_string_u64() (64 bytes, patch)
2013-06-04 02:43 CEST, Peter Stuge
Details
Fix sr_si_string_u64() test cases. from Paul's patch (3.23 KB, patch)
2013-06-04 11:58 CEST, Peter Stuge
Details
Format decimals correctly in sr_si_string_u64() (2.25 KB, patch)
2013-06-04 11:58 CEST, Peter Stuge
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Uwe Hermann 2013-03-09 11:08:00 CET
sr_si_string_u64() and thus also sr_samplerate_string() and other functions that use it don't handle all possible input correctly.

For example, sr_samplerate_string(1234) correctly yields "1.234 kHz", but sr_samplerate_string(1004) yields "1.4 kHz" instead of "1.004 kHz".
Comment 1 Peter Stuge 2013-06-01 22:16:25 CEST
Created attachment 36 [details]
Patch to format decimals correctly in sr_si_string_u64()

The patch is very simple and achieves a string which at least formats a correct value. Maybe more work to strip trailing decimal 0:s could be added on top, should that be desirable.
Comment 2 Paul Sokolovsky 2013-06-03 21:28:54 CEST
Peter's patch still fails tests, as some of test cases expected no trailing zeroes. So, I went to implement trimming them, and turned out that tests are actually inconsistent and some expect trailing zeroes, and some don't. So, I had to patch testcases too after all.

New patch added, it supersedes Peter's.
Comment 3 Paul Sokolovsky 2013-06-03 21:30:40 CEST
Created attachment 37 [details]
This patch fixes missing leading zeroes, trims trailing, and makes sure tests are consistent re: trailing zeroes.
Comment 4 Peter Stuge 2013-06-04 02:41:55 CEST
Created attachment 38 [details]
Fix sr_si_string_u64() test cases.
Comment 5 Peter Stuge 2013-06-04 02:43:01 CEST
Created attachment 39 [details]
Format decimals correctly in sr_si_string_u64()
Comment 6 Peter Stuge 2013-06-04 02:46:40 CEST
Yesterday there was no opinion on trailing zeros so I went for the simplest possible patch.

Today there was consensus to drop trailing zeros, but I didn't like Paul's implementation of formatting so I implemented the formatting in a different way.

Paul's commit to fix the test cases should be applied first!
Comment 7 Paul Sokolovsky 2013-06-04 10:48:47 CEST
The latest attached files don't have any patch hunks in them.
Comment 8 Peter Stuge 2013-06-04 11:58:13 CEST
Created attachment 40 [details]
Fix sr_si_string_u64() test cases. from Paul's patch
Comment 9 Peter Stuge 2013-06-04 11:58:49 CEST
Created attachment 41 [details]
Format decimals correctly in sr_si_string_u64()
Comment 10 Peter Stuge 2013-06-04 11:59:32 CEST
Sorry about that! I've fixed the attachments now.
Comment 11 Uwe Hermann 2013-08-07 16:43:26 CEST
Thanks guys, fixed in ba253f2bb97b7a69ad8b486f411d8d81d33792a4 and 094e6b815972240fbc210f8c5ef2eaa8c989fbc0.

Sorry for the delay.