Icon of Sin / Baphomet
Register | User Profile | Member List | F.A.Q | Privacy Policy | New Blog | Search Forums | Forums Home
Doomworld Forums : Powered by vBulletin version 2.2.5 Doomworld Forums > Classic Doom > Source Ports > Problems with XWE TEXTURE1 lump [title changed]
 
Author
All times are GMT. The time now is 18:35. Post New Thread    Post A Reply
Maes
I like big butts!


Posts: 12759
Registered: 07-06


While trying to repackage some old, notoriously problematic megawads/TCs (Quest for the Necronomicon, Obituary), a common problem I stumbled upon was that both had malformed texture lumps that contained characters beyond the null terminator mark, see example here:

http://www.doomworld.com/vb/showthr...630#post1001630

In that case, a certain texture "YELMIX" was actually stored in the TEXTURE1 lump as "YELMIX 2", where the "space" was actually the null terminator. This problem was present on several textures, and in the case of Quest for the Necronomicon, also in many patch names referred by textures.

When the "YELMIX" texture is recalled from a linedef, most modern source ports (prBoom+ and ZDoom did this) seem to bomb on the grounds that they can't match "YELMIX" with "YELMIX[null]2" with a string comparison (and similarly for other textures). The problem can extend to patches and floors in a similar way.

Yet, vanilla Doom obviously had no problem using the malformed entries with either TC.

My theory is that this is due to how string comparisons are performed in the DOS .exe (fully left-to-right scans until the first null terminator, so the bogus/trailing text is ignored), while modern source ports use modified/optimized string comparisons which however are fooled into believing that strings mismatch (due to encountering valid characters beyond the real, left-most null terminator).

Now, Chocolate Doom in particular, since it's aiming at reproducing the DOS executable's behavior as closely as possible, should probably implement a string comparison function that is guaranteed to behave like the DOS one (e.g. perform no optimizations like starting from the end of a length-limited comparison).

Since texture name matching is done in R_CheckTextureNumForName (char *name) the "offending" library function should be strncasecmp, used as follows:

code:
if (!strncasecmp (textures[i]->name, name, 8) )


Now, in some optimized implementations this might return a false negative on two strings "YELMIX[null]2" and "YELMIX", since it will attempt starting from the 8th character, while unoptimized ones will correctly give a positive. At least that's what I'm imagining.

What's more disturbing is the knowledge that there probably are more such modifications out there, plagued by the same problem which went under the radar for years, which can only be fixed by cleaning up the TEXTURE1 lumps, renaming patch and floor names manually or by making source ports tolerant/modifying their string comparison functions.

Last edited by Maes on 08-15-11 at 19:05

Old Post 08-14-11 00:27 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
natt
Junior Member


Posts: 248
Registered: 05-11


I was not able to reproduce your results in prboom-plus. It uses strncasecmp(foo, bar, 8), and I made a test wad with specifically malformed sidedef texture names but it read them properly as I would expect it to if strncasecmp worked right.

Old Post 08-14-11 01:00 #
natt is offline Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


Doing specific tests witn MinGW also seem to work OK for me, yet in practice me and other people who tested my Obituary repackaging had problems with malformed TEXTURE1 lumps (the malformed text was in TEXTURE1 only, not in the sidedefs themselves), which went away as soon as I corrected the texture name definitions. Maybe it has to do with how texture names are read from the lump during texture initialization.

Compare the behavior of Chocolate Doom and prBoom+ on my current (fixed names) and my initial repackaging of Obituary.

In particular, go to MAP11 and look for the yellow key door. With the unfixed version, you will get a HOM in prBoom+ (where the yellow skull textures are supposed to be) and a crash with error from chocolate doom, complaining it cannot find YELMIX.

These problems were even worse in Quest for the Necronomicon, which also had malformed patch names inside the TEXTURE1 lump.

Last edited by Maes on 08-14-11 at 01:16

Old Post 08-14-11 01:09 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
natt
Junior Member


Posts: 248
Registered: 05-11


First test only used malformed texture names in SIDEDEFS. Created another test wad including TEXTUREx resource with malformed texture names in there. Still can't reproduce any problem at all.

Old Post 08-14-11 01:18 #
natt is offline Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


This means that you DON'T stumble upon a HOM in the situation described above? (using the obticfixold.zip download)

The offending door is on MAP11, X: 296, Y: -1496

I stumbled upon it with prBoom+ 2.5.1.1, from the binary installer. BTW, even DoomBuilder has trouble displaying the YELMIX texture if using the unfixed version (displays "unknown texture" if I leave it as is).

Old Post 08-14-11 01:20 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
natt
Junior Member


Posts: 248
Registered: 05-11



Maes said:
This means that you DON'T stumble upon a HOM in the situation described above? (using the obticfixold.zip download)


No, I do get that error. But I'm unable to reconcile it with your explanation of the problem, as testing strncasecmp() by itself it seems to work properly, and my attempts to create a minimal test case wad failed.

Old Post 08-14-11 01:24 #
natt is offline Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


Well, I pretty much gave up on that explanation after checking it out myself with MinGW. However the problem remains, as does the fact that vanilla Doom apparently has had no problem with it all these years.

Perhaps the discrepancy isn't created at the point that I indicated?

The only certain aspect is that when reading the texture lumps, C/C++ ports just do a raw struct copy without sanitizing the strings, so the malformed strings surely do get in memory intact, with bad terminators and all. But this should not be enough to foil strncasecmp...in theory. At least it doesn't affect the MinGW version....

Last edited by Maes on 08-14-11 at 01:35

Old Post 08-14-11 01:26 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
natt
Junior Member


Posts: 248
Registered: 05-11


I examined obticfix.wad (HOM version)with a hex editor, and in that wad, YELMIX is actually "YELMIX 2", with the space being a space, not a null terminator. I also examined obtic1.wad (the real original) in a hex editor, and in that version YELMIX is actually "YELMIX\x002".

You must have broken the texture lump by editing it in XWE or a similarly braindead editor.

Old Post 08-14-11 01:35 #
natt is offline Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


Heh then it's a XWE problem after all. I will to see if copying the original lump intact (hopefully without it being silently auto-edited) will fix this. Still....I think it would be better if the lump contained no such bogosity at all.

Edit: yup, copying the lump directly by avoiding any triggering whatsoever of XWE's texture editor seemed to fix it. The bad thing is that it affects ALL strings at the same time, even those that went totally untouched, so once you trigger this, you must either fix everything or use a raw copy. Either way, it's still a bad practice to leave uncleaned strings in there.

Last edited by Maes on 08-14-11 at 01:59

Old Post 08-14-11 01:41 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
natt
Junior Member


Posts: 248
Registered: 05-11



Maes said:
Heh then it's a XWE problem after all.


There was a post not too long ago that explained it, but I'm too lazy to search, so I'll just describe it here:

On loading the texture lump into memory, XWE takes any NUL characters in any texture name and replaces them with spaces.
On saving the texture lump, XWE takes any trailing spaces and replaces them with NUL.

Old Post 08-14-11 01:47 #
natt is offline Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
GreyGhost
Why don't I have a custom title by now?!


Posts: 8837
Registered: 01-08


Here is Gez's explanation


Maes said:
Heh then it's a XWE problem after all. I will to see if copying the original lump intact (hopefully without it being silently auto-edited) will fix this. Still....I think it would be better if the lump contained no such bogosity at all.
I've found DeuTex to be a handy tool for sanitizing TEXTURE* and PNAMES lumps, just hope that problem doesn't extend to the SIDEDEFS lumps.

Old Post 08-14-11 05:43 #
GreyGhost is offline Profile || Blog || PM || Email || Search || Add Buddy IP || Edit || Quote
Quasar
Moderator


Posts: 6128
Registered: 08-00


XWE is also known to replace perfectly valid PWADs with statistically random garbage, amongst other things, so this really comes as no shock to me whatsoever.

Old Post 08-14-11 08:29 #
Quasar is offline Profile || Blog || PM || Email || Homepage || Search || Add Buddy IP || Edit || Quote
Graf Zahl
Why don't I have a custom title by now?!


Posts: 7787
Registered: 01-03


People should really stop using XWE...

Old Post 08-14-11 12:05 #
Graf Zahl is offline Profile || Blog || PM || Email || Search || Add Buddy IP || Edit || Quote
natt
Junior Member


Posts: 248
Registered: 05-11



Quasar said:
statistically random garbage


Perhaps the XWE codebase could be re-purposed as a cryptographic random number generator?

Old Post 08-14-11 23:20 #
natt is offline Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


Well, if the problem is really in THAT ONE SPOT only, I could give my 1337 (albeit rusty) Delphi skillz a polish and fix it to do proper C-like (aka null-terminated, left-to-right scanned) string parsing and solve everybody some grief.

Old Post 08-15-11 00:58 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
fraggle
Filled with the code of Doom


Posts: 7778
Registered: 07-00


So I've read through the entire thread, and I'm not sure what (if anything) I need to fix...

Is there a specific identifiable difference between Vanilla and Chocolate or not? Does it happen on all platforms, or only some?

Old Post 08-15-11 12:38 #
fraggle is online now Profile || Blog || PM || Email || Homepage || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


No no, it's all OK. Turned out that if anything needs fixing, it's XWE's (broken) parsing of strings in the TEXTURE1 lump (pulls out dusty Delphi 7 IDE from uni).

Old Post 08-15-11 13:09 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
Gez
Why don't I have a custom title by now?!


Posts: 11396
Registered: 07-07


If you could make it so that XWE does not constantly rewrite the opened wad, completely fucking it up in the process if it's opened in another application (including another instance of XWE), that would be just gravy.

Old Post 08-15-11 14:15 #
Gez is online now Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06


Heh I just re-opened the XWE project, and remembered why I had just kind of gave up on continuing/improving it: it's missing some dependencies (including what appears to be a custom-written string manipulation unit) and an external ZipMaster one, other than being designed for Delphi 5 and not 7. It's a chore just getting it to compile as-is.

Edit: the source Csabo released is not complete: it's missing A LOT of external dependencies, some of which are quite hard to find (e.g. the CodeMax highlighting editor's Delphi CodeMax_TLB headers) or were probably custom-written by Csabo himself ('Stringz' string utilities, 'Evaluation' numerical evaluation routines etc.) and not included in the released source code.

I can attempt to recreate some of the missing routines or replace the ZipMaster with a more recent versions, but even then it's missing just too much stuff to even get to compile.

Last edited by Maes on 08-15-11 at 16:34

Old Post 08-15-11 14:20 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
AlexMax
Senior Member


Posts: 1115
Registered: 01-03



Maes said:
I can attempt to recreate some of the missing routines or replace the ZipMaster with a more recent versions, but even then it's missing just too much stuff to even get to compile.

Why not contribute to SLADE 3 instead?

Old Post 08-15-11 22:42 #
AlexMax is offline Profile || Blog || PM || Email || Search || Add Buddy IP || Edit || Quote
Gez
Why don't I have a custom title by now?!


Posts: 11396
Registered: 07-07



Maes said:
Heh I just re-opened the XWE project, and remembered why I had just kind of gave up on continuing/improving it: it's missing some dependencies (including what appears to be a custom-written string manipulation unit) and an external ZipMaster one, other than being designed for Delphi 5 and not 7. It's a chore just getting it to compile as-is.



Csabo said:
It's written in Delphi 5 (I know, ancient). One quick note: if PNGImage/CodeMax/Zip handlers give you trouble, you're probably better off commenting out anything that deals with those parts. I don't think those are essential to WAD editing, and could be replaced with other libraries.


So yeah.


AlexMax said:
Why not contribute to SLADE 3 instead?


The real problem is to get people to stop recommending XWE. Because as long as it's promoted despite having been left unmaintained for five years now (last release was in 2006), people will keep using it, and stumble unto its bugs, that won't ever be patched.

Old Post 08-16-11 14:53 #
Gez is online now Profile || Blog || PM || Search || Add Buddy IP || Edit || Quote
Maes
I like big butts!


Posts: 12759
Registered: 07-06



Gez said:
Stuff Csabo said about commenting out "troublesome" parts of the code


Easier said than done. Most of the CodeMax parts may be indeed commentable out/$IFDEF-ed with the preprocessor, and the Zip/Gif/Png parts are easy enough to recover online and include in the project. But some parts are so ingrained in the code that you can't simply "comment them out" without a major rewrite, and unless you use the particular versions used by Csabo, you will need to rewrite some parts anyway.

The trouble is that there are some string manipulation and number evaluation stuff that are NOT included in the source and from the looks of it are custom-written libraries, and are used all over the code. These need recreation based on their names and guesstimating their possible functionality (e.g. KeepFromLeft, PosR, Zero, Comma,IsNumber, SafeVal and others), as there's no documentation about them, nor any mention that they were part of a known library.

The fact that it's for Delphi 5 is not such aa proplem per-se (Delphi 7 handles that fine), as much as all that missing stuff.


Gez said:
The real problem is to get people to stop recommending XWE. Because as long as it's promoted despite having been left unmaintained for five years now (last release was in 2006), people will keep using it, and stumble unto its bugs, that won't ever be patched.


Well, WinTex lived on for much longer ;-)

Old Post 08-16-11 15:28 #
Maes is offline Profile || Blog || PM || Homepage || Search || Add Buddy IP || Edit || Quote
All times are GMT. The time now is 18:35. Post New Thread    Post A Reply
 
Doomworld Forums : Powered by vBulletin version 2.2.5 Doomworld Forums > Classic Doom > Source Ports > Problems with XWE TEXTURE1 lump [title changed]

Show Printable Version | Email this Page | Subscribe to this Thread

 

Forum Rules:
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts
HTML code is OFF
vB code is ON
Smilies are OFF
[IMG] code is ON
 

< Contact Us - Doomworld >

Powered by: vBulletin Version 2.2.5
Copyright ©2000, 2001, Jelsoft Enterprises Limited.