The International Simutrans Forum

 

Author Topic: C++11-only feature in simworld.cc  (Read 870 times)

0 Members and 1 Guest are viewing this topic.

Offline isidoro

  • Devotee
  • *
  • Posts: 1128
C++11-only feature in simworld.cc
« on: September 29, 2018, 02:15:47 PM »
In simworld.cc:
Code: [Select]
4562         std::string savename = filename;
4563         savename.back() = '_';
4564
method back of basic_string is a C++11 feature and prevent the file from compiling in state-of-the-art Linux with -ansi flag.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9418
  • Languages: De,EN,JP
Re: C++11-only feature in simworld.cc
« Reply #1 on: September 29, 2018, 02:25:32 PM »
Ok, how would do this?
savename[savename.length()-1] = '_';

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2596
  • Languages: EN
Re: C++11-only feature in simworld.cc
« Reply #2 on: September 29, 2018, 03:00:27 PM »
Quote
state-of-the-art Linux with -ansi flag
These two are kind of contradictory. It is "state-of-the-art" but you are running it in a mode targeting a specification from 1998. A Linux build with GCC from 2008 could quite possibly produce the same results.
Quote
Ok, how would do this?
savename[savename.length()-1] = '_';
Yes that seems about right.

This still assumes the last byte is always a single byte code point. If it is a multi-byte code point the result will likely be an invalid string.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: C++11-only feature in simworld.cc
« Reply #3 on: September 29, 2018, 04:37:44 PM »
This still assumes the last byte is always a single byte code point. If it is a multi-byte code point the result will likely be an invalid string.
Since users have no input over the file name extension for save games, that assumption should be solid in this case. There might be similar problems elsewhere in the code, though.

Offline isidoro

  • Devotee
  • *
  • Posts: 1128
Re: C++11-only feature in simworld.cc
« Reply #4 on: September 29, 2018, 09:48:52 PM »
Ok, how would do this?
savename[savename.length()-1] = '_';

Can savename be the empty string?

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2596
  • Languages: EN
Re: C++11-only feature in simworld.cc
« Reply #5 on: September 30, 2018, 07:01:56 AM »
Quote
Can savename be the empty string?
I am guessing not since apparently it will always at least have the extension at the end of it.

That said for code quality one really should be manipulating strings by glyph position rather than arbitrarily by byte.
« Last Edit: September 30, 2018, 08:40:20 AM by DrSuperGood »

Offline Dwachs

  • DevTeam, Coder/patcher
  • Administrator
  • *
  • Posts: 4555
  • Languages: EN, DE, AT
Re: C++11-only feature in simworld.cc
« Reply #6 on: September 30, 2018, 04:34:20 PM »
why not
Code: [Select]
savegame.append( "_" )
? This would ignore all the trouble with multi-byte, empty strings, and what not.

Offline ACarlotti

  • *
  • Posts: 354
Re: C++11-only feature in simworld.cc
« Reply #7 on: September 30, 2018, 07:24:40 PM »
It makes the filename longer, which could cause problems. Prissi referred to this issue on the other thread:
... and will not lead into troubles should appending .part make a too long filename for that system

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2596
  • Languages: EN
Re: C++11-only feature in simworld.cc
« Reply #8 on: October 01, 2018, 09:01:26 AM »
If it is a temporary file name, one could generate one from a time stamp. Fixed length so no concern of becoming too long. When the save is moved, one could rename it properly.

Offline prissi

  • Developer
  • Administrator
  • *
  • Posts: 9418
  • Languages: De,EN,JP
Re: C++11-only feature in simworld.cc
« Reply #9 on: October 01, 2018, 01:53:49 PM »
savename cannot be empty, otherwise fopen woudl fail. An empty filename will result in a file ".sve", so there is for sure at least four bztes in savename.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: C++11-only feature in simworld.cc
« Reply #10 on: October 01, 2018, 04:30:16 PM »
If it is a temporary file name, one could generate one from a time stamp. Fixed length so no concern of becoming too long. When the save is moved, one could rename it properly.
Well, strictly speaking, any filename longer than five (four?) characters/bytes (including extension) could be too long. And that is assuming the player can save any games at all.

For autosave/load to work, there must be at least room for a 14 byte filename. Since the temp file does not actually need an extension, that would be enough for some kind of timestamp.

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2596
  • Languages: EN
Re: C++11-only feature in simworld.cc
« Reply #11 on: October 01, 2018, 07:11:31 PM »
Quote
Well, strictly speaking, any filename longer than five (four?) characters/bytes (including extension) could be too long. And that is assuming the player can save any games at all.
What OS that is supported has those limits? Windows the path length is 255 characters odd, extendable to over 6,000 odd if a better file system API was used.

In any case on Windows an appropriate temporary folder could be used which should have more free characters available. If not then chances are other applications would be breaking as well.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: C++11-only feature in simworld.cc
« Reply #12 on: October 01, 2018, 07:43:56 PM »
What OS that is supported has those limits? Windows the path length is 255 characters odd, extendable to over 6,000 odd if a better file system API was used.

That is for the entire path, not just the file name. So the OS I had in mind was Windows, although I guess any OS can give these kinds of problems if the user is insane enough about naming directories.

Offline isidoro

  • Devotee
  • *
  • Posts: 1128
Re: C++11-only feature in simworld.cc
« Reply #13 on: October 01, 2018, 10:36:20 PM »
Why not let the task to the OS?  At least for Linux, you have mkstemp.  Don't know (and don't want to know  ;D )about other OSes.

Offline DrSuperGood

  • Dev Team
  • Devotee
  • *
  • Posts: 2596
  • Languages: EN
Re: C++11-only feature in simworld.cc
« Reply #14 on: October 01, 2018, 11:31:22 PM »
Quote
Why not let the task to the OS?  At least for Linux, you have mkstemp.  Don't know (and don't want to know   )about other OSes.
Because the file might need to persist. From what I understand temporary file IO is intended for temporary storage locations for bulk data and is largely a legacy left from pre virtual memory days where one could not abuse a memory mapped page file for extra storage space in an application with potentially terabytes of virtual memory.

What is happening here is that a file is temporarily written to another location before being moved to its intended persistent location. This is a safety in case Simutrans crashes during save so that existing saves only get overwritten with complete saves. OSes might implement file moves more efficiently than a delete/truncate then write since at a low level they may not need to move the file data around, only the file system entries to the file data.

Offline Ters

  • Coder/patcher
  • Devotee
  • *
  • Posts: 5447
  • Languages: EN, NO
Re: C++11-only feature in simworld.cc
« Reply #15 on: October 02, 2018, 05:52:14 AM »
Why not let the task to the OS?  At least for Linux, you have mkstemp.  Don't know (and don't want to know  ;D )about other OSes.

In this case, we want to create the file in the same directory, or at least on the same volume. mkstemp gives no control of this. Saving temporarily in the save directory has two major advantages. Firstly, as DrSuperGood also points out, one does not need to move the data from one volume to another, which takes time (and increases risk of failure). The OS only needs to update the directory entries. (It could create links across volumes, but this would be confusing. If the save game directory is on a thumb drive, you want the data to be on the thumb drive.) Secondly, the temp file can be found just as easily by the user if the move operation fails. The move operation may then be completed manually. The user may also send an incomplete save when reporting a bug in the saving.

The only drawback is that the volume needs to have room for two save games, even if only temporarily: the one being written, and the previous one. However, I haven't had such problems with insufficient storage space since the days of (not so) floppy disks.