Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Introducing Malicious Code Reviews

paulbiggar.blogspot.com

Post reply on HN
Print view View original

1–10 of 15 posts

Re: Introducing Malicious Code Reviews

#3

Post by pbiggar »

tptacek wrote: post #2

The best you can come up with is how tangled the code in a hand-written lexer looks? I may have skimmed a bit towards the end, but, did you actually find a bug?

Since its practically the entire contents of the file, its hard to do better.

As to the presence of a bug, impossible to tell. Which is the point.

Re: Introducing Malicious Code Reviews

#5

Post by tptacek »

pbiggar wrote: post #3
tptacek wrote: post #2

The best you can come up with is how tangled the code in a hand-written lexer looks? I may have skimmed a bit towards the end, but, did you actually find a bug?

Since its practically the entire contents of the file, its hard to do better. As to the presence of a bug, impossible to tell. Which is the point.

So, I do a lot of code review --- a job hazard --- and we tend to score them based on whether we find anything. You may want to find a softer target.

Re: Introducing Malicious Code Reviews

#7

Post by mindviews »

tptacek wrote: post #2

The best you can come up with is how tangled the code in a hand-written lexer looks? I may have skimmed a bit towards the end, but, did you actually find a bug?

Didn't look like he pointed out any bugs - but that's not really the main point of a code review. It's all well and good for the compiler to understand the code and have it execute properly, but is it human-readable? Or maintainable? Having looked at the source file, I'd say that particular method is neither.

Re: Introducing Malicious Code Reviews

#8

Post by cschneid »

tptacek wrote: post #6
cschneid wrote: post #4

Is the PHP community open to refactorings of the code? Do they have the tests in place to make that possible?

Why would they want to refactor their lexer? They're hard to get right in the first place.

Mostly, I'm curious about my second question. What's the state of testing the core of PHP? I'm totally unfamiliar with the implementation.

I understand that it may be dumb to refactor the lexer, but it gets less dumb if it's supported by hundreds of tests, and far more dumb if it's based on testing over time.

Re: Introducing Malicious Code Reviews

#9

Post by huhtenberg »

tptacek wrote: post #2

The best you can come up with is how tangled the code in a hand-written lexer looks? I may have skimmed a bit towards the end, but, did you actually find a bug?

That code is really bad. Dunno about you, but to me this handcrafted entanglement looks like the crap that any professional programmer should be ashamed of:

  if ( .. )
  {
     for ( .. )
     {
  label:
        ...
     }
  }
  else
  {
     if ( .. )
     {
       ..
     }
     else
       goto label;
     ..
   }
The actual code is even worse than this as there's a second goto that jumps from the top for into one of else blocks at the bottom from where the control can go back to for via another goto ... I sort of can see how this code came about, but there is absolutely no excuse for checking in something this f*cked up.

If they wanted to show off, they should've just used the coroutines :-)

Post reply on HN