View Issue Details

IDProjectCategoryView StatusLast Update
0000641LDMud 3.5Implementationpublic2018-01-30 03:59
Reporterzesstra Assigned Tozesstra  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformx86_64OSMacOS XOS Version10.5.x
Target Version3.5.0Fixed in Version3.5.0 
Summary0000641: save_object() should never write NaN or INF into savefiles.
DescriptionIf a float is actually too large to be represented and it is saved in save_object()/save_value(), a restore with restore_object()/restore_value() will fail (not only for the float, but for the complete savefile). A similar issue is "NaN".
restore_svalue() expects numbers to start with [0-9,-], but sprintf("%.12e") with my libc will print infinity as 'inf' (and NaN probably as 'nan'/'NaN').
C99 specifies that sprintf uses "[-]inf" or "[-]infinity" for infinity and a string starting with "nan" for NaN when printing floating point numbers.

One idea is, to define a new savefile format:
a) with a special first character as a tag for floats. The downside is, that the savefiles gets a little bit bigger and as less readable for humans.
b) never write the native float representation ("%.12e") into the savefile, only use separate mantissa+exponent. The downside is, the savefiles get even less readable for humans as well. Additionally I am not completely sure, what implications this has for portability. Right now we have only on floating point implementation, but maybe that changes in the future and creating yet another savefile format then is not something I look forward to...
TagsNo tags attached.

Relationships

related to 0000627 resolvedzesstra LDMud 3.3 Floating point numbers on 64bit platforms may be saved incorrectly 

Activities

fufu

2009-05-26 10:03

manager   ~0001163

Thanks for separating this issue out from 0000627.

I was wondering how NaNs and infs would end up in the save file in the first place, ignoring 0000627 -- the driver seems to be careful to not create such values and turns them into errors instead.

zesstra

2009-05-27 03:32

administrator   ~0001167

Mhmm. Very good point. And preventing overflows/underflows in LPC is a good idea. I guess you are right, we might just define any case of a float being inf or NaN a bug somewhere else.
The only downside of that is, that savefiles may be written, which can't be restored. We could check that in save_svalue() and raise an error in that case to prevent writing illegal savefiles...

zesstra

2009-10-28 05:15

administrator   ~0001567

I think, I concur with Fuchur, that 'infinity' and 'NaN' should not be in savefiles in the first place, if they are, there is a problem somewhere else.
So I suggest to close this. Any more opinions?

Bardioc

2009-10-30 03:00

reporter   ~0001573

I think its a good idea to not serialize the values for NaN and Infinity as they are somewhat special. so is NaN always != NaN ... I wonder how this is handled after restoring the objects value.

From my opinion, making it an error is correct, as it obviously is one. I cannot think of somebody really taking advantage of saving NaN or Inf.

zesstra

2009-10-30 04:11

administrator   ~0001574

> I wonder how this is handled after restoring the objects value.
Well, it is not handled, because we can't restore savefiles with such values. ;-) You just get an error about the savefile having an illegal format (or such).

As Fuchur writes, the driver usually does not create those values in the first place (0000627 was special because due to architecture issues a float was mis-interpreted during save_svalue()). But currently, we don't have an active check in save_svalue() for these.

zesstra

2010-03-14 16:43

administrator   ~0001799

Do we need such a check (for NaN, infinity) in save_svalue() at the time we write the savefile?

Coogan

2011-02-24 00:46

reporter   ~0002036

I say: Yes, we need such a check, and the try to save Inf or NaN values should result in an error.

In a mud, I can't imagine a use case where it is needed to write Inf or NaN into a savefile. I even think that a mudlib's need to store such a value in a savefile does more seem to be a design error.

zesstra

2012-12-09 01:59

administrator   ~0002175

Last edited: 2012-12-09 01:59

I agree concerning the need of these values Coogan, but the argument is: there should be no situation the driver writes NaN or infinity into an svalue. Therefore no check in save_svalue() is necessary. If the driver creates such an svalue, that itself is an error that has to be fixed.
The only argument is: we want the check because we are paranoid. ;-)

zesstra

2014-02-23 22:05

administrator   ~0002230

I will add an assert in save_svalue() checking that the double written are finite. That will prevent invalid savefiles even if the driver has an error somewhere else. That should make all of us paranoid people happy. ;-)

ldmud-bugs

2014-02-23 22:15

viewer   ~0002231

Fix committed in revision e8ad384d5c9f26f2675ff7bd2c1fee66772a201f to master branch (see changeset 937 for details). Thank you for reporting!

zesstra

2018-01-29 18:59

administrator   ~0002314

Fix committed in revision e8ad384d5c9f26f2675ff7bd2c1fee66772a201f to master branch (see changeset 1437 for details). Thank you for reporting!

zesstra

2018-01-29 21:57

administrator   ~0002365

Fix committed in revision e8ad384d5c9f26f2675ff7bd2c1fee66772a201f to master branch (see changeset 2765 for details). Thank you for reporting!

zesstra

2018-01-30 03:59

administrator   ~0002416

Fix committed in revision e8ad384d5c9f26f2675ff7bd2c1fee66772a201f to master branch (see changeset 3850 for details). Thank you for reporting!

Issue History

Date Modified Username Field Change
2009-05-26 03:03 zesstra New Issue
2009-05-26 03:08 zesstra Relationship added related to 0000627
2009-05-26 10:03 fufu Note Added: 0001163
2009-05-27 03:32 zesstra Note Added: 0001167
2009-10-28 05:15 zesstra Note Added: 0001567
2009-10-28 05:15 zesstra Status new => feedback
2009-10-30 03:00 Bardioc Note Added: 0001573
2009-10-30 04:11 zesstra Note Added: 0001574
2010-02-25 15:23 zesstra Summary restore_object() should be able to restore floats having the value 'infinity' and 'NaN'. => save_object() should never write NaN or INF into savefiles.
2010-03-14 16:43 zesstra Note Added: 0001799
2011-02-19 17:54 zesstra Target Version 3.3.720 => 3.3.721
2011-02-24 00:46 Coogan Note Added: 0002036
2012-12-09 01:59 zesstra Note Added: 0002175
2012-12-09 01:59 zesstra Status feedback => new
2012-12-09 01:59 zesstra Note Edited: 0002175
2014-02-23 22:05 zesstra Note Added: 0002230
2014-02-23 22:05 zesstra Assigned To => zesstra
2014-02-23 22:05 zesstra Status new => assigned
2014-02-23 22:15 ldmud-bugs Source_changeset_attached => ldmud.git master e8ad384d
2014-02-23 22:15 ldmud-bugs Note Added: 0002231
2014-02-23 22:15 ldmud-bugs Assigned To zesstra => ldmud-bugs
2014-02-23 22:15 ldmud-bugs Status assigned => resolved
2014-02-23 22:15 ldmud-bugs Resolution open => fixed
2014-02-23 22:19 zesstra Project LDMud 3.3 => LDMud 3.5
2014-02-23 22:20 zesstra Assigned To ldmud-bugs =>
2014-02-23 22:20 zesstra Product Version 3.3.718 =>
2014-02-23 22:20 zesstra Fixed in Version => 3.5.0
2014-02-23 22:20 zesstra Target Version 3.3.721 => 3.5.0
2014-02-23 22:20 zesstra Assigned To => zesstra
2018-01-29 18:59 zesstra Source_changeset_attached => ldmud.git master e8ad384d
2018-01-29 18:59 zesstra Note Added: 0002314
2018-01-29 21:57 zesstra Source_changeset_attached => ldmud.git master e8ad384d
2018-01-29 21:57 zesstra Note Added: 0002365
2018-01-30 03:59 zesstra Source_changeset_attached => ldmud.git master e8ad384d
2018-01-30 03:59 zesstra Note Added: 0002416