Multiple drag from JUCE app broken in win


#1

I think the problem is in the addBytesToPointer loop over the files. But its possible the error is actually with the number of bytes returned by
String::copyToUTF16. The docs say returns the number of bytes copied to the buffer, including the terminating null character. But when adding the filenames together there is no terminating NULL.

Anyway, i’m sure you’ll figure it out! (I’m on the June 30 GIT, but I did a quick check of the commit logs to see if there was anything added)

[code]
static HDROP createHDrop (const StringArray& fileNames)
{
int totalBytes = 0;
for (int i = fileNames.size(); --i >= 0;)
totalBytes += CharPointer_UTF16::getBytesRequiredFor (fileNames[i].getCharPointer()) + sizeof (WCHAR);

HDROP hDrop = (HDROP) GlobalAlloc (GMEM_MOVEABLE | GMEM_ZEROINIT, sizeof (DROPFILES) + totalBytes + 4);

if (hDrop != 0)
{
    LPDROPFILES pDropFiles = (LPDROPFILES) GlobalLock (hDrop);
    pDropFiles->pFiles = sizeof (DROPFILES);
    pDropFiles->fWide = true;

    WCHAR* fname = reinterpret_cast<WCHAR*> (addBytesToPointer (pDropFiles, sizeof (DROPFILES)));

WCHAR debug=fname;
for (int i = 0; i < fileNames.size(); ++i)
{
const int bytesWritten = fileNames[i].copyToUTF16 (fname, 2048);
fname = reinterpret_cast<WCHAR
> (addBytesToPointer (fname, bytesWritten+sizeof(WCHAR))); // manually add the 2 bytes needed for the null
}

    *fname = 0;
    GlobalUnlock (hDrop);
}

return hDrop;

}[/code]


#2

copyToUTF16 is fine.

From MSDN DROPFILES Structure

[quote] The data that follows is a double null-terminated list of file names [/quote] so my fix seems fine


#3

I don’t understand… I think my code is absolutely fine. It looks like your change will just leave two null characters after every filename, which would mean that only the first one will be used. My code leaves a single null after each filename, and then two at the end, which is what it’s supposed to do. (Or am I totally misunderstanding?)


#4

HMMM, you are right, I took a deeper look at MSDN and its supposed to look like this

c:\temp1.txt\0c:\temp2.txt\0\0

Which your original code looks like it should do, but the debugger is giving me

c:\temp1.txtc:\temp2.txt\0

i’ll take a better look tomorrow…


#5

Actually I took another look

for (int i = 0; i < fileNames.size(); ++i) { DBG(fileNames[i].length()); const int bytesWritten = fileNames[i].copyToUTF16 (fname, 2048); fname = reinterpret_cast<WCHAR*> (addBytesToPointer (fname, bytesWritten+sizeof(WCHAR))); }

I get the filenames length == 49, but the bytesWritten returns back with 98, I would expect bytes written to be 100? (49 characters long + a NULL char == 50 *sizeof(WCHAR) == 100)

So maybe my initial suspicion with copyToUTF16 was right?

Here’s a little test case

		String utfTest("atest");
		DBG(utfTest.length());
		HeapBlock<CharPointer_UTF16::CharType> charBuffer(2048);
		int bytesWritten=utfTest.copyToUTF16(charBuffer, 2048/sizeof(CharPointer_UTF16::CharType));
		DBG(bytesWritten);

Outputs on my version(I do need to checkout the most recent version from GIT though, so go ahead and yell ‘I fixed it’ if you have!)

EDIT: Same output numbers from GIT snapshot

PC and Mac Build
5 and 10


#6

Ah, yes, you’re right, copyToUTF16 doesn’t seem to be returning the correct value. The problem is actually in CharacterFunctions::copyWithDestByteLimit(), which should return this:

return getAddressDifference (dest.getAddress(), startAddress) + sizeof (typename DestCharPointerType::CharType);


#7

Yup, that makes sense, tnx Jules!


#8

Hi Jules,
I assume the copyToUTF16 fix is in the tip, but what do you think about moving this fix also into the juce_153 branch? Bugs like this always seem to pop-up once in a while, one more serious than others. Because you are always working on newer code in the trunk it’s not always safe for us to update to the tip, i’ve done this a couple of times and a few times it fixed a current bug but introduced new ones.
Wouldn’t be a good thing to have a stable branch (1_53) which doesn’t get updated with new features but do get the bug fixes? I know it’s more work but would make more sense. Your codebase is getting so big and with additions like ios and android support it’s much harder to maintain a bug free main branch.


#9

I’ve not put it into the tip yet, because I’m in the middle of the “Jucequake” re-organisation at the moment. When that’s done I’ll probably do a new release soon afterwards, and then might indeed think about stable branches, etc., which will be more straightforward when everything is more modularised.


#10

Ok, i understand you want to do this after your ‘quake’, as long as it is not followed by some aftershocks :wink: