search

staktrace / cafebabe

A java class file parser
43 stars 12 forks source link

Descriptor parser #19

Closed bluejekyll closed 2 years ago

bluejekyll commented 2 years ago

This adds descriptor parsing for both method_info (MethodDescriptor) as well as field_info (FieldType).

Fixes: #18

bluejekyll commented 2 years ago

@staktrace, let me know if there's anything else you'd like to see here. Otherwise I think this is complete.

staktrace commented 2 years ago

Sorry for the delay. I did start looking at it and had some concerns about the string copies which I generally try to avoid. I haven't had a chance to look more closely yet, but hopefully will get to it in the next day or two.

bluejekyll commented 2 years ago

If the string copies are a really big concern, I can attempt to factor them out. That's going to take some work based on the current lifetime's with Chars, etc, but should be feasible. I'll try and refactor for that.

bluejekyll commented 2 years ago

ok, I removed any unnecessary string allocation. it makes the parsing logic ever so slightly more complex, but in the end not too bad.

let me know if you think there is a simpler way to achieve this.

bluejekyll commented 2 years ago

Thanks for the reviews. I think the last change covers your concerns.

staktrace commented 2 years ago

Hm I was looking at the code some more and I realized that the descriptor parsing doesn't properly handle some cases, like arrays with more than 255 dimensions. Those are supposed to be treated as invalid but the descriptor parser is ok with it.

bluejekyll commented 2 years ago

I can put together a patch to catch that case. See: #21