After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 728722 - Setting number format details appear wrong in Help, section 10.3.4. Counters Book Options Tab
Setting number format details appear wrong in Help, section 10.3.4. Counters ...
Status: RESOLVED FIXED
Product: GnuCash
Classification: Other
Component: Business
git-master
Other Linux
: Normal normal
: ---
Assigned To: gnucash-core-maint
gnucash-core-maint
Depends on:
Blocks:
 
 
Reported: 2014-04-22 12:12 UTC by Mike Evans
Modified: 2018-06-29 23:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch Add format token lli in Help Counters Book Options Tab (2.76 KB, patch)
2016-01-21 05:35 UTC, Chris Good
committed Details | Review

Description Mike Evans 2014-04-22 12:12:56 UTC
The number format is stated to be "%li" but it should be "%lli" on my Fedora 18 system.  This bug is for Linux but it may apply to other OS's too.

When logging to stout the error is:

WARN <qof.engine> [qof_book_get_counter_format()] Invalid counter format string. Format string: 'MEC-'%li'' Counter: 'gncBill' Error: 'Invalid length modifier and/or conversion specifier ('li''), it should be: lli')
Comment 1 Chris Good 2015-09-26 22:24:54 UTC
Hi Mike,

What version of GnuCash are you using?

On my Ubuntu 12.04 GnuCash 2.4.12: %li is OK, %lli fails
eg
	Customer number format 	C%li	OK 	Created C1
			       	C%lli	BAD	Created 000002
	Vendor number	       	V%li	OK	Created V2
				V%04li	OK	Created V0003
				V%04i	BAD	Created 000004
	Bill number		B%li	OK	Created	B2
				B%lli	BAD	Created 000003
				B%04li	OK	Created B0004
Regards, Chris Good

Note to myself:
When updating documentation, add to Help, Ch7 Business Features, Initial Setup, Business Preferences, a reference to
Book Options 'Counters' for formats of CustNo etc
Comment 2 Mike Evans 2015-09-27 10:30:06 UTC
Hi Chris

Using gnucash-maint now but git-master at the time.

Looking back at the #gnucash IRC logs from about [Thursday 05 Jun 2014] [20:21:35] I see that this may have been due the differences in the definition of long long int in Ubuntu and Fedora libraries.

Checking again with the latest commit 12f760228 with Fedora18 I still get the same error message:

Of course Fedora18 is waaay out of date so this may not be an issue on newer Fedora releases.  Maybe Geert can check this out as I know he runs a more recent Fedora.  I think everyone runs a more recent version than me.

I can no longer compile master branch as Fedora18 is too old.  I really, really need to upgrade.
Comment 3 Geert Janssens 2015-09-28 16:37:11 UTC
On my Fedora 22 box I need to use %li and not %lli. But perhaps this is more a matter of 32-bit vs 64-bit systems. I have installed the 64-bit edition of Fedora. What editions of the OS (Fedora and Ubuntu respectively) do you have ?
Comment 4 John Ralls 2015-09-28 19:02:03 UTC
qof_book_validate_counter_format compares the l or ll against G_GINT64_FORMAT, which is hard-coded to "lli" except on Windows where it's "I64i". That means that following the docs will raise the error Mike noted in make check, which is apparently the only place that the function is called.

FWIW the POSIX standard macro is PRIx64.

As for actually passing the parameter to printf or scanf, "lli" is correct for 32-bit glibc but not 64-bit where it should be "li", though I've encountered at least one 64-bit printf that takes either one (don't remember where). It's always "lli" on Apple, so probably also on BSD. 

Where in the code is this option actually used? I can't find it.
Comment 5 Chris Good 2015-09-28 23:42:36 UTC
Hi Geert, Ubuntu 12.04 64 bit.
Comment 6 Mike Evans 2015-09-29 09:42:39 UTC
My Fedora18 is 32 bit.

In my use-case it's only for formatting the invoice/bill numbers in Properties->Book Options->Counters set Bill Number format to to MEC-%lli .

Since the libraries are not under our control I thought this should be mentioned in the documentation, although I've never seen any support requests about this.  I guess there aren't many people running ancient 32 bit Fedora out there to make it worth any greater effort.
Comment 7 Geert Janssens 2015-09-29 15:54:21 UTC
(In reply to John Ralls from comment #4)
> Where in the code is this option actually used? I can't find it.

The code is used indirectly by several business objects by calling qof_book_increment_and_format_counter

For example invoices use it in gncInvoiceNextID (see https://github.com/Gnucash/gnucash/blob/maint/src/engine/gncInvoice.c#L2161 )

A quick search shows this is also used by the employee, job, customer, order and vendor objects.
Comment 8 Geert Janssens 2015-09-29 16:00:43 UTC
(In reply to Mike E from comment #6)
> My Fedora18 is 32 bit.
> 
> In my use-case it's only for formatting the invoice/bill numbers in
> Properties->Book Options->Counters set Bill Number format to to MEC-%lli .
> 
> Since the libraries are not under our control I thought this should be
> mentioned in the documentation, although I've never seen any support
> requests about this.  I guess there aren't many people running ancient 32
> bit Fedora out there to make it worth any greater effort.

I do think it's worth mentioning. An "ancient 32 bit Fedora" is not the only exception. On Windows users are supposed to use %I64i instead of %li or %lli. [1]

It was a very unfortunate decision to follow plain printf syntax when this counter formatting code was originally implemented. Understandable at the time (most OS's were still 32-bit and Windows may not even have been on the support horizon perhaps), yet unfortunate.

It is how it is however and until someone cleans up that code and writes support for a universal number format, it's best we document the current quirks clearly. That will certainly simplify life of the people interested in using this.

[1] see the comment in the parser code here:
https://github.com/Gnucash/gnucash/blob/maint/src/libqof/qof/qofbook.c#L562
Comment 9 John Ralls 2015-09-29 16:10:09 UTC
ISTM the obvious fix is to just change what we pass to g_strdup_printf() to PRIx64 (not G_GINT64_FORMAT, which is wrong on 64-bit Linuxes) instead of raising a warning, especially one that's incomprehensible to non-programmers.

OTOH, why are we using int64_t counters? Surely just int is sufficient, and then we don't need to pass a length qualifier at all.
Comment 10 Geert Janssens 2015-09-29 16:32:26 UTC
(In reply to John Ralls from comment #9)
> ISTM the obvious fix is to just change what we pass to g_strdup_printf() to
> PRIx64 (not G_GINT64_FORMAT, which is wrong on 64-bit Linuxes) instead of
> raising a warning, especially one that's incomprehensible to non-programmers.
> 
Perhaps I'm slow here so I'll ask for some clarification:
- the user enters a format string in the business properties (File->Properties->Counters). Something like "INV-%.3lli"
- When required, this format string is used to generate an invoice number, like "INV-005"
- Before it does so, it tests if the format has a valid int64 format specifier (li, lli or I64i,...) based on what is set in G_GINT64_FORMAT

So as the code is now, the user has to know what is the proper format specifier for his particular platform.

When you mention "PRIx64", is that what you expect the user to use as format specifier then ? Like in the counter dialog he should enter something such as
"INV-%.3PRIx64i" ?

Or do you mean to change the test for valid int64 format to use PRIx64 instead of G_GINT64_FORMAT. What is the user to enter in the counter dialog in this case ?


> OTOH, why are we using int64_t counters? Surely just int is sufficient, and
> then we don't need to pass a length qualifier at all.

I have no idea why the original author chose this. Perhaps to be sure the number will always be formatted exactly the same way on all different platforms (for users that switch platforms on one book) ? Perhaps that can easily be done with a normal int as well. I'm fine with that, as long as whatever we choose is properly documented.
Comment 11 John Ralls 2015-09-29 21:34:46 UTC
Use PRIx64, which is a POSIX macro, instead of G_GINT64_FORMAT and instead of checking just figure that the user wants the appropriate int64_t format (which is what PRIx64 will be defined to) and stick that in the format string at the appropriate location.

Derek's the original author, and he thinks it was probably just to avoid casting in and out of the KVP int64 slot.

If we switch the representation we'd have to discard the "l" or "ll" in the user's format string in gnc_book_validate_counter_format_internal() to prevent breakage with existing formats. In the short term it's easier just to make the substitution and leave the rest alone.
Comment 12 Chris Good 2016-01-21 05:35:03 UTC
Created attachment 319483 [details] [review]
Patch Add format token lli in Help Counters Book Options Tab

Also add setting up the Help Counters Book Options to the list of things to do when setting up the business features.
Comment 13 Frank H. Ellenberger 2016-01-26 22:39:24 UTC
Review of attachment 319483 [details] [review]:

commit fd63aab
Comment 14 Frank H. Ellenberger 2016-01-26 22:47:42 UTC
As the current state is now documented, I reassign it to Business.
Comment 15 Geert Janssens 2016-03-17 21:47:31 UTC
I have just pushed some changes to the underlying code that ensures gnucash will accept several format specifiers regardless of the platform on which it runs. The ones explicitly tested for are
li
lli
I64i
In all of these cases, gnucash will do "the right thing". That is, the counter will be set up as needed for the platform on which gnucash is run. The user can select any of the above ones and stick to that one. The datafile can even be opened on different platforms and it will continue to work (where before you would have to change the counter format each time you changed from Windows to linux or the other way around).

I intend to fix the documentation for this as well in a minute.
Comment 16 Geert Janssens 2016-03-17 22:16:29 UTC
I have made some tweaks in the help texts. This should be sufficient to close this bug (finally).
Comment 17 John Ralls 2018-06-29 23:29:59 UTC
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=728722. Please update any external references or bookmarks.