Running under a debugger, the function arrfree / arrfree$() does cause an exception when freeing the memory of the pointer-array (GlobalFree). The actual problem is the function arralloc/arralloc$(), which does write to the unallocated array element
[esi+mcnt*4+4]. Furthermore the loop condition should be checked before accessing the array to avoid problems when mcnt == 0.
Quotearralloc proc mcnt:DWORD
; ----------------------------------------------------------------
; return values = handle of pointer array or 0 on allocation error
; ----------------------------------------------------------------
push esi
mov eax, mcnt ; load the member count into EAX
add eax, 1 ; correct for 1 based array
lea eax, [0+eax*4] ; multiply it by 4 for memory size
invoke GlobalAlloc,GMEM_FIXED,eax
mov esi, eax
test eax, eax ; if allocation failure return zero
jz quit
mov eax, esi
mov ecx, mcnt
mov DWORD PTR [eax], ecx ; write count to 1st member
xor edx, edx
@@:
add edx, 1 ; write adress of null string to all members
mov [eax+edx*4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
cmp edx, ecx
jle @B <----------- should be JL or JB
mov eax, esi ; return pointer array handle
quit:
pop esi
ret
arralloc endp
i seem to recall we had troubles with something related a couple years ago
http://www.masmforum.com/board/index.php?topic=17159.0 (http://www.masmforum.com/board/index.php?topic=17159.0)
It could also be shortened a little bit:
arralloc proc mcnt:DWORD
; ----------------------------------------------------------------
; return values = handle of pointer array or 0 on allocation error
; ----------------------------------------------------------------
mov eax, mcnt ; load the member count into EAX
push eax
lea eax, [eax*4+4] ; correct for 1 based array, multiply it by 4 for memory size
invoke GlobalAlloc,GMEM_FIXED,eax
test eax, eax ; if allocation failure return zero
pop ecx
jz quit
mov DWORD PTR [eax], ecx ; write count to 1st member
@@:
mov [eax+ecx*4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
dec ecx
ja @B
; return pointer array handle
quit:
ret
arralloc endp
:biggrin: a little tidier...
mov ecx,mcnt
lea eax,[4*ecx+4]
push ecx
INVOKE GlobalAlloc,GMEM_FIXED,eax
test eax,eax
pop ecx
jz quit
Oops, you are right, the pop must come before the jz :redface:
well - maybe not - there is always LEAVE
but - it's clearer if you pop first
it wouldn't cost anything to preserve EBX, ESI, or EDI for that value
The problem I have had with these algos is that every time someone had a problem you suggested a mod then never got any feedback so I was always left in the lurch as to whether they worked properly.
Quote from: dedndave on November 04, 2013, 11:32:50 AM
it wouldn't cost anything to preserve EBX, ESI, or EDI for that value
What's the advantage of using ebx instead of ecx?
i guess the main advantage is, it's easier to read :)
the cost is - you push/pop EBX (or ESI or EDI) at entry and exit
which is ok - it replaces a push/pop on ECX - so no cost, really
arralloc proc uses ebx mcnt:DWORD
; ----------------------------------------------------------------
; return values = handle of pointer array or 0 on allocation error
; ----------------------------------------------------------------
mov ebx, mcnt ; load the member count into EBX
lea eax, [ebx*4+4] ; correct for 1 based array, multiply it by 4 for memory size
invoke GlobalAlloc,GMEM_FIXED,eax
test eax, eax ; if allocation failure return zero
jz quit
mov DWORD PTR [eax], ebx ; write count to 1st member
@@:
mov [eax+ebx*4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
dec ebx
ja @B
; return pointer array handle
quit:
ret
arralloc endp
Quote from: hutch-- on November 04, 2013, 01:26:49 PM
The problem I have had with these algos is that every time someone had a problem you suggested a mod then never got any feedback so I was always left in the lurch as to whether they worked properly.
to give a feed back now, I would use the following version, which avoids partial flag stalls, access the memory in ascending order and also handle the case mcnt==0:
stackframe off
align 16
arralloc2 proc mcnt:DWORD
; ----------------------------------------------------------------
; return values = handle of pointer array or 0 on allocation error
; ----------------------------------------------------------------
mov eax, [esp+4] ; load the member count into EAX
push eax
lea eax, [eax*4+4] ; correct for 1 based array, multiply it by 4 for memory size
invoke GlobalAlloc,GMEM_FIXED,eax
test eax, eax ; if allocation failure return zero
pop edx ; load the member count into EDX
jz quit
mov [eax],edx ; write count to 1st member
and ecx,0 ; loop alignment
jmp @1
@@: db 3Eh ; DS prefix for alignment
mov [eax+ecx*4+4], OFFSET d_e_f_a_u_l_t__n_u_l_l_$
add ecx,1
@1: cmp ecx,edx
jb @B
; return pointer array handle
quit:
ret 4
arralloc2 endp
stackframe onGenerally, how should one report bugs in the MASM32 SDK? Over the time I've got the impression that a subforum is not that efficient...
Its probably the case that I am not always that efficient. I have to find the originals, the test pieces then enough brain space to digest the problem and I don't always have the time. I do have a good look at the posted problems but don't always have the time/brain space to work on it.
as i recall, we had a lot of participation in the old forum on this set of routines
and we resolved the problem as it was reported
but this is a slightly different issue, i think
qWord is a great coder
but........
if they call the function with 0, they get what they deserve :lol:
aligning short loops yields little advantage, if any
Quote from: dedndave on November 06, 2013, 05:01:57 AMif they call the function with 0, they get what they deserve :lol:
you should think out of the box ;)
Quote from: dedndave on November 06, 2013, 05:01:57 AMaligning short loops yields little advantage, if any
the cost are minimal in this case, thus we should use that potential.
Quote from: qWord on November 06, 2013, 06:02:05 AM
you should think out of the box ;)
:biggrin:
giving you a hard time - lol
well - to be fair.....
if you are going to test for 0, do it before allocating any space :P
arralloc proc uses ebx mcnt:DWORD
; ----------------------------------------------------------------
; return values = handle of pointer array or 0 on allocation error
; ----------------------------------------------------------------
mov ebx, mcnt ; load the member count into EBX
test ebx, ebx
mov eax, ebx
jz quit
Quote from: dedndave on November 06, 2013, 06:47:54 AMif you are going to test for 0, do it before allocating any space
As being a dynamic data structure, it should be allowed to create a empty array (at least that is my humble opinion).
Quote from: qWord on November 06, 2013, 07:15:21 AMAs being a dynamic data structure, it should be allowed to create a empty array
Semi-dynamic, maybe. Example from hlhelp.chm:
LOCAL hArray :DWORD
......
mov hArray, arralloc$(2048)
......
mov hArray, arrealloc$(hArray,4096)Slightly more dynamic:
include \masm32\MasmBasic\MasmBasic.inc ; download (http://masm32.com/board/index.php?topic=94.0)
Init ; ## create a dynamic string array ##
Dim My$()
xor ecx, ecx
.Repeat
Let My$(ecx)=Str$("String %i", ecx)
inc ecx
.Until ecx>=20000
Inkey "ok"
Exit
end startBut in both cases, a "true" empty array makes little sense. You wouldn't create it if you didn't want to use it, right?
P.S.: Your version is better than mine. I had not thought of accessing memory from bottom to top. I wonder, however, if the direction really matters. Testbed? ;)
Here it is:
Intel(R) Celeron(R) M CPU 420 @ 1.60GHz (SSE3)
9947 kCycles for 5 * down
10274 kCycles for 5 * up
10205 kCycles for 5 * down
10284 kCycles for 5 * up
10155 kCycles for 5 * down
10212 kCycles for 5 * up
35 bytes for down
38 bytes for up
OK, I didn't consider the reallocation strategy, which change the pointer/handle. Regardless that, under certain conditions it can make sense to have empty containers.