News:

Simutrans Sites
Know our official sites. Find tools and resources for Simutrans.

C++11-only feature in simworld.cc

Started by isidoro, September 29, 2018, 02:15:47 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

isidoro

In simworld.cc: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.

prissi

Ok, how would do this?
savename[savename.length()-1] = '_';

DrSuperGood

Quotestate-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.
QuoteOk, 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.

Ters

Quote from: DrSuperGood on September 29, 2018, 03:00:27 PMThis 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.

isidoro

Quote from: prissi on September 29, 2018, 02:25:32 PM
Ok, how would do this?
savename[savename.length()-1] = '_';

Can savename be the empty string?

DrSuperGood

#5
QuoteCan 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.

Dwachs

why not

savegame.append( "_" )

? This would ignore all the trouble with multi-byte, empty strings, and what not.
Parsley, sage, rosemary, and maggikraut.

ACarlotti

It makes the filename longer, which could cause problems. Prissi referred to this issue on the other thread:
Quote from: prissi on September 24, 2018, 02:19:51 PM... and will not lead into troubles should appending .part make a too long filename for that system

DrSuperGood

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.

prissi

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.

Ters

Quote from: DrSuperGood 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.
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.

DrSuperGood

QuoteWell, 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.

Ters

Quote from: DrSuperGood on October 01, 2018, 07:11:31 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.

isidoro

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.

DrSuperGood

QuoteWhy 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.

Ters

Quote from: isidoro 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.

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.