4961 add registry content viewer #4792
Conversation
…gnature but incorrect data
|
closing PR till I finish clean up since codacy seems to be somewhat satisfied |
|
Should be ready to review. |
rcordovano
requested review from
dannysmyda and
eugene7646
and removed request for
dannysmyda
| try { | ||
| content.read(data, 0x0, content.getSize()); | ||
| } catch (TskCoreException ex) { | ||
| logger.log(Level.WARNING, "Failed to read file content.", ex); |
There was a problem hiding this comment.
Shouldn't we exit here?
There was a problem hiding this comment.
Returning there sounds like a reasonable thing to do if the reading of the data fails. It wasn't being done in the original code I was integrating, but that is no indication it shouldn't be done. I will make this change and test to ensure it works, unless I hear otherwise.
There was a problem hiding this comment.
I don't really know what the right answer is so you should probably seek the opinion of @rcordovano.
| try { | ||
| file.read(header, 0x0, Math.min(0x4000, file.getSize())); | ||
| } catch (TskCoreException ex) { | ||
| logger.log(Level.WARNING, "Failed to read file content", ex); |
There was a problem hiding this comment.
I guess same question, shouldn't we exit if there was an exception while trying to read data from the file? Is there a point in trying to continue and to pass incomplete/gibberish data to other classes?
There was a problem hiding this comment.
Same answer, I have no idea if there is a point.
| } | ||
|
|
||
| // the number of line endings before the start,end points | ||
| int startRows = (startByte - (startByte % bytesPerLine)) / (3 * bytesPerLine); |
There was a problem hiding this comment.
We should probably make the "3" into a constant, since we are using it in multiple places.
| } else { | ||
| sb.append(valString); | ||
| } | ||
| if (sb.length() > 48) { |
There was a problem hiding this comment.
I think we should also make constants out of these, I would have no idea what 45 or 48 is if I had to debug the code.
There was a problem hiding this comment.
Alright, the constants might be a bit vague in name like "MAX_VALUE_STRING_LEN" or something like that but I agree even vaguely named constants would be easier to understand than the raw numbers.
|
I have finished the review and testing. As soon as the minor code review things are taken care of - the PR is good to go. |
3 participants
Making PR now to start working on what I expect to be many Codacy complaints