Opened 2 years ago

Closed 7 months ago

#14685 closed enhancement (fixed)

wxTextInputStream doesn't support 64bit integers

Reported by: peterix Owned by:
Priority: normal Milestone:
Component: base Version: stable-latest
Keywords: 64bit streams simple Cc: zuban32rus@…
Blocked By: Blocking:
Patch: yes

Description

I'm reading some unix timestamps from files and using a 64bit integer for them. wxTextInputStream can't do it.

Attachments (3)

64_txtstrm.patch download (5.3 KB) - added by zuban32 7 months ago.
64txtstrm.patch download (5.5 KB) - added by zuban32 7 months ago.
txtstrm.cpp.patch download (548 bytes) - added by zhchbin 7 months ago.
Fix compile error of msw: Use string format instead of macro concatenation.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 2 years ago by vadz

  • Keywords 64bit streams simple added
  • Status changed from new to confirmed

Yes, it'd be nice to add Read64[S]() methods (and the corresponding Write64() to wxTextOutputStream).

Any patches doing this would be definitely welcome.

comment:2 Changed 8 months ago by zuban32

  • Cc zuban32rus@… added
  • Patch set

comment:3 Changed 8 months ago by vadz

Thanks! This looks mostly good but a few small things prevent it from being applied:

  1. Using %llu and %lld is not quite portable, we have wxLongLongFmtSpec which should be used instead. Or, even simpler, just use wxString::operator<<() overloads taking wx[U]LongLong_t.
  2. Speaking of wxString, you should probably also use its To[U]LongLong() methods instead of dealing with wxStrto[u]ll() directly. At the very least this frees you from the need to use .c_str() explicitly (which is not quite the right thing to do here...).
  3. Bonus points for updating the documentation! But to make it perfect, could you please add @since 3.1.0 to the newly added functions?
  4. I'm not sure what is the "Now we can rewrite if we want all *32 functions" comment about, could you please explain?

P.S. I guess you must be one of GSoC students, could you please let me know (here or privately by writing to vadim-at-wxwidgets-dot-org) your name so that we could map it to these patches when we consider your application? Thanks!

comment:4 Changed 7 months ago by csomor

sorry for jumping in, but shouldn't Read64S return a wxInt64 instead of a wxUint64 ?

Changed 7 months ago by zuban32

comment:5 Changed 7 months ago by zuban32

my mistake, patch updated

comment:6 Changed 7 months ago by vadz

Err, what about the other points mentioned in the comment:3?

comment:7 follow-up: Changed 7 months ago by zuban32

can wxLongLongFmtSpec be used both for signed and unsigned?

comment:8 in reply to: ↑ 7 Changed 7 months ago by vadz

Replying to zuban32:

can wxLongLongFmtSpec be used both for signed and unsigned?

If you look at how it's defined/used, you can see that it doesn't include "d" part. So you just use it with "u" instead in the unsigned case.

comment:9 Changed 7 months ago by zuban32

I read the definition, but I didn't see any unsigned case there. So where exactly should I place 'u'?

comment:10 Changed 7 months ago by astronothing

The wxLongLongFmtSpec only contains the length part of the format, not the specifier. As seen in http://www.cplusplus.com/reference/cstdio/printf/ the specifier indicates whether it's signed or unsigned (e.g. "d" or "u").

For an example of how it is used, you can check out wxString's operator<< which takes a rhs argument of either wxLongLong_t or wxULongLong_t. Here are the definitions: https://github.com/wxWidgets/wxWidgets/blob/master/include/wx/string.h#L2078

Happy coding :)

Changed 7 months ago by zuban32

comment:11 Changed 7 months ago by zuban32

new patch, all you've told is fixed

comment:12 Changed 7 months ago by vadz

Thanks, looks good, will commit soon.

comment:13 Changed 7 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [76171]) Add wxInt64 support to wxText{Input,Output}Stream.

Add explicit Read64[S]() and Write64() methods.

Closes #14685.

Changed 7 months ago by zhchbin

Fix compile error of msw: Use string format instead of macro concatenation.

comment:14 Changed 7 months ago by zhchbin

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen this ticket due to the patch broke msw build, AFAICS. :)

Here is the build output on VS2010:

src\common\txtstrm.cpp(402): error C2308: concatenating mismatched strings

Concatenating wide "%" with narrow "I64"

src\common\txtstrm.cpp(402): error C2308: concatenating mismatched strings

Concatenating wide "%" with narrow "u"

Patch fixing this problem attached.

comment:15 Changed 7 months ago by vadz

Oops, thanks. I've fixed this in operator<< but somehow overlooked it here, thanks again for noticing and fixing.

comment:16 Changed 7 months ago by VZ

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [76172]) Compilation fix in wchar_t build after r76171.

Don't concatenate narrow wxLongLongFmtSpec with wide strings, just don't use
wxT() at all.

Closes #14685.

Note: See TracTickets for help on using tickets.