On Thu, 14 Oct 2010 13:01:36 -0400, Andrej Mitrovic <n...@none.com> wrote:
I'm not sure if this is a legitimate bug or one of my brainfarts.
I know that if I'm using a foreach loop with a char array as a reusable
datatype I definitely have to create a copy if I want to store it to a
string.
But this code is a little more subtle, check it out (This is Windows
only because it uses the registry, sorry):
module mymodule;
import std.stdio : writeln, write;
import std.windows.registry;
void main()
{
Key HKLM = Registry.localMachine;
Key SFW = HKLM.getKey("software");
string[] names;
foreach (Key key; SFW.keys())
{
string name = key.name();
// string name = key.name().idup; // workaround for the issue
names ~= name;
}
writeln("results:");
foreach (name; names)
{
write(name, ", ");
}
}
The results are quite unexpected. The strings get overwritten with each
other, and in my case the results are similar to this:
Sun Microsystems, Sun Micros, Sun , Sun Micr, Sun, Sun Mic,...
And it goes like that for a hundred or so values, then switches to the
next name and writes more garbage like that.
If I use .idup, the problem goes away. What I don't understand is why
assigning a string to a string isn't safe in this case? They're both
immutable, so I was expecting the contents of the strings never to
change.
If it's not a bug, it certainly is a subtle issue. The original foreach
loop was quite big, and it took some time to figure out the problem. Are
we *always* supossed to be using .idup in a foreach loop? Of course, the
Key key variable is reused in the foreach loop, so I guess this has
something to do with the results.
I'm not familiar with std.windows.registry, but there are two things here.
First, unequivocally, this is a bug. If key.name is returning an
immutable(char)[], and later on that value is being overwritten, this is a
violation of the type system (immutable data must never change again).
Second, I think the bug may not be that it's violating immutability, but
rather that it's typed key.name as string. It could probably be
const(char)[]. Essentially, I think from the behavior described that the
foreach loop is reusing the name buffer for each iteration of the loop.
This is probably to save extra heap allocations in case you don't use them
after the foreach loop is over. I think your workaround is the correct
way to deal with it.
This is a common problem in defining an opApply loop which streams data --
do you make things safe or efficient? The only reasonable solution IMO is
to make them efficient, because safety can be had by duping the data.
-Steve
