Fix issue 18084 - tempCString type should not change size when used in unittests.

[schveiguy]

Fixes memory corruption introduced when (ironically) trying to support dip25: #4746

Instead of the original mechanism (which was to slice the buffer with an enum), I just added padding when in unittest mode to make sure the size is the same. Very hard to unittest this, since unittests actually change the layout in the original! I'm open to suggestions.

ping @WalterBright

[dlang-bot]

Thanks for your pull request, @schveiguy!

Bugzilla references

Auto-close	Bugzilla	Description
✓	18084	[REG2.072] tempCString buffer size is unittest-versioned

[schveiguy]

Crap, I didn't target stable.

[wilzbach]

You can change the branch when you click on edit. Top right

[schveiguy]

OK, I'll try it. Will it rebase automatically?

[wilzbach]

OK, I'll try it. Will it rebase automatically?

Nope. Try:

git rebase --onto upstream/stable

[schveiguy]

OK, I think I got it. This may cause problems when merging back to master, because this whole file was refactored.

[schveiguy]

ping, this is pretty trivial, and is a memory corruption bug. We should fix it ASAP.

@MartinNowak @WalterBright @andralex @klickverbot

[schveiguy]

Ah nuts. So the master version that I first targeted got merged into stable 😆

So I will rework this again back the way it was. At least there won't be any merge conflicts!

[schveiguy]

OK, that should do it.

@wilzbach the problem was that master had refactored the file, so when I retargeted stable, there were conflicts when trying to rebase. Then the master branch was merged into stable, so there were conflicts again! In any case, it should be good now (I hope).

[andralex]

@MartinNowak target to stable?

[jacob-carlborg]

Instead of the original mechanism (which was to slice the buffer with an enum), I just added padding when in unittest mode to make sure the size is the same. Very hard to unittest this, since unittests actually change the layout in the original! I'm open to suggestions.

@schveiguy can you instantiate TempCStringBuffer with a larger type than the default to get the same behavior without having to change the size in unit test mode?

[wilzbach]

target to stable?

It's already targeted at stable:

@wilzbach the problem was that master had refactored the file, so when I retargeted stable, there were conflicts when trying to rebase. Then the master branch was merged into stable, so there were conflicts again! In any case, it should be good now (I hope).

I'm glad you managed to work it out & sorry that you had to rebase this twice - bad timing :/

[schveiguy]

@jacob-carlborg I think the idea is to utilize all the existing uses of tempCString to exercise the allocation scheme. I’m not sure it makes a difference or that it will possibly find more errors, but I also didn’t write the original. Perhaps @WalterBright can shed some light.

[jacob-carlborg]

I think the idea is to utilize all the existing uses of tempCString to exercise the allocation scheme

Aha, ok. I didn't look at tempCString, I only looked at TempCStringBuffer. I see that tempCString requires a character of some sort while it looks like TempCStringBuffer would work with any type.

[schveiguy]

yeah, TempCStringBuffer was actually a voldemort type before a recent refactoring, so it's logical that no template constraints were added to it.

[wilzbach]

FWIW this is an excellent example why StdUnittest would be really useful (see #5927).

[MartinNowak]

Thanks, I've been running into this misterious tempCStringW bug in my io library.
schveiguy/io@6ab5a63