Ticket #1499 (closed defect: fixed)

Opened 9 years ago

Last modified 9 years ago

mcview: Partially responsible for this is signed/unsigned mismatch

Reported by: dborca Owned by: angel_il
Priority: major Milestone: 4.7.0-pre2
Component: mcview Version: 4.7.0-pre1
Keywords: Cc:
Blocked By: Blocking:
Branch state: Votes for changeset: slavazanko

Description

Partially responsible for this is signed/unsigned mismatch in the following test:
if (search_end - search_start > edit->search->original_len && ...)
which I changed to
if (search_end > search_start + edit->search->original_len && ...)

Change History

comment:1 Changed 9 years ago by angel_il

  • severity changed from no branch to on review

branch: 1499_search_broken (parent: master)
changeset: 2be9b7761dca373cb7f55602a5ae3b81aa0a7665

comment:2 Changed 9 years ago by angel_il

  • Votes for changeset set to angel_il

comment:3 Changed 9 years ago by angel_il

  • Owner set to angel_il
  • Status changed from new to accepted

comment:4 Changed 9 years ago by iNode

  • Votes for changeset changed from angel_il to angel_il iNode
  • severity changed from on review to approved

comment:5 Changed 9 years ago by angel_il

  • Status changed from accepted to testing
  • Votes for changeset changed from angel_il iNode to commited-master
  • Resolution set to fixed
  • severity changed from approved to merged

comment:6 Changed 9 years ago by angel_il

  • Status changed from testing to closed

comment:7 follow-up: ↓ 8 Changed 9 years ago by andrew_b

I disagree with this patch.

search_end > search_start + edit->search->original_len

The potential oveflow is here.

comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 9 years ago by dborca

Replying to andrew_b:

I disagree with this patch.

search_end > search_start + edit->search->original_len

The potential oveflow is here.

No shit! How about giving a real solution instead of just "disagree"ing? Your disagreement do not fix the backwards search problem. It just blocks it.

Be honest. You disagree because I already mentioned the overflow problem, not because you sensed it. The potential overflow was mentioned several times now already. By me. Having problems searching for info? Where were you to mention the non-potential-but-always-present problem of signed/unsigned mismatch? It's trivial.

And no, I'm not looking to be agreeable. I'm just a prick. But I'm looking to fix things as best as I can. For real-world issues, for real-world users, in real-world situations.

I'm pretty sure you're a bunch of stellar programmers in MC team. And you don't need help. Strangely though, I don't see many valuable contributions around here, just requests. How come?

PS: I know how to fix the overflow problem, but I won't fix it, because I'm sure YOU will. Good luck!

comment:9 in reply to: ↑ 8 Changed 9 years ago by andrew_b

Replying to dborca:

No shit!

Please calm down!

How about giving a real solution instead of just "disagree"ing? Your disagreement do not fix the backwards search problem. It just blocks it.

Wrong. I wrote my comment after your patch was commited to the master branch. So my disagreement cannot blocks anything.

Be honest.

Sure.

You disagree because I already mentioned the overflow problem, not because you sensed it.

No. You're flatting yourself.

The potential overflow was mentioned several times now already. By me.

Really?

Having problems searching for info?

Be so fine, give me a link.

But I'm looking to fix things as best as I can.
[...]
PS: I know how to fix the overflow problem, but I won't fix it because I'm sure YOU will.

Strange. You know about the overflow problem in this piece of code, but you didn't fix it. Strange...

You are

comment:10 Changed 9 years ago by angel_il

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:11 Changed 9 years ago by angel_il

branch: 1499_fix
changeset: 548f92eb8c0543a7785989d346d326342867530d

comment:12 Changed 9 years ago by slavazanko

  • Votes for changeset changed from commited-master to slavazanko
  • severity changed from merged to on review

comment:13 Changed 9 years ago by angel_il

  • Status changed from reopened to closed
  • Resolution set to fixed

fixed in DEV_mcviev2 #1557

Note: See TracTickets for help on using tickets.