Advertisement

What's wrong with this?

Started by March 05, 2002 09:32 PM
17 comments, last by Draxis 22 years, 6 months ago
Nevermind, string2 isn''t missing a nul, string1 just has an extra carriage return character.
Calling strlen even once in a string-comparison function is generally completely unnecessary, as you have to loop through each string in order to compare them in any case, and 2 calls to strlen in the function is pretty much going to double the time the function uses.

Anyway, here''s my tack:
  int compstr (const char* str1, const char* str2) {    // Returns 0 if the strings match    while (*str1 == *str2) {        if (!*str1)            return 0;        ++str1;        ++str2;    }    return 1;}  


-Neophyte

- Death awaits you all with nasty, big, pointy teeth. -
Advertisement
quote: Original post by Neophyte
Calling strlen even once in a string-comparison function is generally completely unnecessary, as you have to loop through each string in order to compare them in any case, and 2 calls to strlen in the function is pretty much going to double the time the function uses.

Anyway, here''s my tack:
    int compstr (const char* str1, const char* str2) {    // Returns 0 if the strings match    while (*str1 == *str2) {        if (!*str1)            return 0;        ++str1;        ++str2;    }    return 1;}    


-Neophyte

- Death awaits you all with nasty, big, pointy teeth. -

That''s an overread if str1 is longer than str2.
quote: Original post by ZealousElixir
3 calls to strlen vs. 2 calls to strlen and two temp variables?

Maybe if you wanted to go all out, but the performance gain with strings of reasonable size is negligible, and the other code is a lot prettier. Also, if you''re using the methods that don''t require strlen at all, you''re doing three comparisons per loop iteration...a bit excessive.


Excessive in what sense? You''re going to do all comparisons anyway, only they''ll be in the library functions, and with some extra (probably negligible) iteration overhead.

Normally, yeah, I''d do the strlen thing too (tho I''d store them in temp variables) just because it''s more clear, and quicker. But he''s obviously going for agressive, possibly academic optimization, otherwise he would''ve left the thing alone ;-)
try the Boyer Moore exact pattern matching algorithm

;long int BMHBinsearch(long int startpos,long int*lpSource,long int srcLngth,long int* lpSubStr,long intsubLngth);
; #########################################################################

.486
.model flat, stdcall ; 32 bit memory model
option casemap :none ; case sensitive

.code

; #########################################################################

BMHBinsearch proc startpos:DWORD,
lpSource:DWORD,srcLngth:DWORD,
lpSubStr:DWORD,subLngth:DWORD

; ------------------------------------------------------------------
; This algorithm is related to a Horsepool variation of a Boyer
; Moore exact pattern matching algorithm. It only uses the bad char
; shift and increments the source if the character is in the table
; ------------------------------------------------------------------

LOCAL cval:DWORD
LOCAL shift_table[256]:DWORD

push ebx
push esi
push edi

mov ebx, subLngth

cmp ebx, 1
jg @F
mov eax, -2 ; string too short, must be > 1
jmp BMHout
@@:

mov esi, lpSource
add esi, srcLngth
sub esi, ebx
mov edx, esi ; set Exit Length

; ----------------------------------------
; load shift table with value in subLngth
; ----------------------------------------
mov ecx, 256
mov eax, ebx
lea edi, shift_table
rep stosd

; ----------------------------------------------
; load decending count values into shift table
; ----------------------------------------------
mov ecx, ebx ; SubString length in ECX
dec ecx ; correct for zero based index
mov esi, lpSubStr ; address of SubString in ESI
lea edi, shift_table

xor eax, eax

Write_Chars:
mov al, [esi] ; get the character
inc esi
mov [edi+eax*4], ecx ; write shift for each character
dec ecx ; to ascii location in table
jnz Write_Chars

; -----------------------------
; set up for main compare loop
; -----------------------------
mov ecx, ebx
dec ecx
mov cval, ecx

mov esi, lpSource
mov edi, lpSubStr
add esi, startpos ; add starting position

jmp Main_Loop

; %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Inc_Shift:
cmp esi, edx ; test for exit condition
jg MisMatch
inc esi ; increment shift

Pre_Main:
mov ecx, cval ; reset counter in compare loop
xor eax, eax ; zero EAX

Main_Loop:
mov al, [esi+ecx]
cmp al, [edi+ecx] ; cmp characters in ESI / EDI
jne Get_Shift ; if not equal, get next shift
dec ecx
jns Main_Loop

jmp Matchx

Get_Shift:
cmp ebx, shift_table[eax*4] ; cmp subLngth to char shift
jne Inc_Shift ; if not equal jump to Inc_Shift
lea esi, [esi+ecx+1] ; add bad char shift
cmp esi, edx ; test for exit condition
jl Pre_Main

jmp MisMatch

; %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Matchx:
sub esi, lpSource ; sub source from ESI
mov eax, esi ; put length in eax
jmp BMHout

MisMatch:
mov eax, -1

BMHout:
pop edi
pop esi
pop ebx

ret

BMHBinsearch endp

; #########################################################################

end
int compstring(const char* str1, const char* str2){   do   {      if (*str1 != *str2) return 0;   } while (*str1++ && *str2++);   return 1;} 
alexk7
Advertisement
Sneftel: Oops. I missed that.
The
  if (!*str1)   return 0;  

should read
  if (!*str1 || !*str2)   return 0;  


-Neophyte

- Death awaits you all with nasty, big, pointy teeth. -
int compstring(const char* str1, const char* str2)
{
do
{
if (*str1 != *str2++) return 0;
} while (*str1++);
return 1;
}




Edited by - BeerNutts on March 6, 2002 1:35:23 PM

My Gamedev Journal: 2D Game Making, the Easy Way

---(Old Blog, still has good info): 2dGameMaking
-----
"No one ever posts on that message board; it's too crowded." - Yoga Berra (sorta)

now that we have optimized this...lets use the standard C/C++ library functions which I''m sure have already been optimized...

  if( strcmp(string1,string2) == 0 ){  return 1;}else{  return 0;}  
Joseph FernaldSoftware EngineerRed Storm Entertainment.------------------------The opinions expressed are that of the person postingand not that of Red Storm Entertainment.

This topic is closed to new replies.

Advertisement