views:

53

answers:

1

Can anybody point out why? I can't see the problem.

String to search: "aassaas"

String to search with: "as"

SEARCHSTRING:
  STMFD SP!, {R4-R7, LR} 

  MOV R6, #0               @Matches found
  MOV R3, #0               @Placeholder     

LOOP: LDRB R4, [R0]        @R4 = String to search
  LDRB R5, [R1]            @R5 = String to search with

  CMP R4, R5               @Do they match?
  ADDEQ R3, R3, #1         @If yes, increase placeholder
  LDREQB R4, [R0, #1]!     @Get next char
  LDREQB R5, [R1, #1]!     @Get next char
  BLNE RESET               @If not, reset placeholder and strings.
                           @R0 is nevertheless initial pos+1
  CMP R5, #0               @Is string to search with at the end?
  ADDEQ R6, R6, #1         @If so, add +1 to matches
  BLEQ RESET               @Reset placeholder and strings.

  CMP R4, #0               @Is the string to search finished?
  BNE LOOP                 @If not, start over.

  MOV R0, R6               @If so, move answer into R0.
  LDMFD SP!, {R4-R7, PC}   @Jump back.

RESET:
  STMFD SP!, {LR}

  CMP R3, #0               @Is the placeholder at 0? (initial position)
  SUBNE R0, R0, R3         @If not, subtract from String to search pos
  SUBNE R1, R1, R3         @And string to be searched pos
  ADDNE R0, R0, #1         @Increment string to search+1 so we don't start at the same spot
  MOVNE R3, #0             @Empty the placeholder

  LDMFD SP!, {PC}          @Jump back
+2  A: 

I don't understand why a) you're writing this in assembler instead of C, and b) why you're not using some routine based on strstr. The most likely scenario is that this is a homework problem, or some other form of learning exercise, so I don't want to give too much away. In any event, there are a couple of problems that I noticed. The first bit I notice is in the RESET routine:

RESET:
  STMFD SP!, {LR}

  CMP R3, #0               @Is the placeholder at 0? (initial position)
  SUBNE R0, R0, R3         @If not, subtract from String to search pos
  SUBNE R1, R1, R3         @And string to be searched pos
  ADDNE R0, R0, #1         @Increment string to search+1 so we don't start at the same spot
  MOVNE R3, #0             @Empty the placeholder

  LDMFD SP!, {PC}          @Jump back

The CMP is unnecessary - consider what the effect of the SUBNE calls will be if R3 is 0, and you'll see that you can perform the subtractions unconditionally. You want to run ADD R0, R0, #1 unconditionally - in fact, this is a big part of the reason you have an infinite loop. If you get to the RESET subroutine, and R3 is 0, then it doesn't change any state. I also notice that the STMFD / LDMFD pair is really not necessary - LR won't be modified in this subroutine, so it doesn't need to go on the stack.

Next, I notice that you're not careful enough about when to terminate your loop. Consider what happens if you give two empty strings as arguments to SEARCHSTRING. Call it with two empty strings as arguments, and single-step through your assembly code to see the problem. The general form of a for loop, when compiled to assembly, will be something like:

for(initial; comparison; increment) {
  body;
}

INITIAL:
    MOV R0, #0         @initialize variables
    B CONDITION        @jump to condition check
BODY:
    LDR R1, [R0]
INCREMENT:             @really, part of the for-loop body.
    ADD R0, R0, #1
CONDITION:
    CMP BLAH, BLAH     @test-condition
    BLT BODY           @restart loop if condition indicates we should do so.

hopefully this will help you to reorganize the code in a more straightforward way.

Aidan Cully
@Aidan Cully: Thanks for the suggestions! This is indeed a homework problem. I have since posting the question solved it by eliminating the conditional ADDNE command by changing it to ADD. Since reading this, however, I noticed that I really do still have egregious flaws in the design.What I did not show was a separate routine that filtered out empty strings because our teacher wanted a something special to happen if the string(s) is(are) empty.Thank you for the tipps! Although the teacher didn't comment on any of the flaws you have shown, I will refactor my code anyway!
SoulBeaver