Fixing Coverity Defects Part Deux #1130
Conversation
| @@ -164,7 +169,7 @@ public virtual string Content | |||
| /// </summary> | |||
| /// <param name="patch"><see cref="Patch"/>.</param> | |||
| /// <returns>The patch content as string.</returns> | |||
| public static implicit operator string(Patch patch) | |||
| public static implicit operator string (Patch patch) | |||
There was a problem hiding this comment.
Mono-style hits LibGit2Sharp for 42 damage points.
|
@Therzok update made, can you please re-review? 🙏 |
|
Looks good to me otherwise 👍 |
|
@Therzok 🙇 thank you sir. @nulltoken you're up: what's your opinion about |
|
This needs a rebase. |
… where the issues can occur.
…or code to the callback invoker.
Done. Not painful - sorry to disappoint 😜 |
| @@ -66,6 +66,11 @@ private int PrintCallBack(GitDiffDelta delta, GitDiffHunk hunk, GitDiffLine line | |||
| PatchEntryChanges currentChange = this[filePath]; | |||
| string prefix = string.Empty; | |||
|
|
|||
| if (currentChange == null) | |||
| { | |||
| return (int)GitErrorCode.NotFound; | |||
There was a problem hiding this comment.
I'm not sure about. This will brutally flow back to libgit2.
Is there even a case when currentChange could actually be null?
There was a problem hiding this comment.
Is there even a case when currentChange could actually be null?
@nulltoken Coverity sure seems to think there is. Perhaps in some kind of repo corruption case? Not sure really. Null checking doesn't seem like a bad thing. Very little overhead and it would be correct to route and error back to the base lib if the value were null.
Another round of fixes for Coverity discovered defects.
Please closely review the NRE fixes, as there are likely better ways to resolve them that I have no considered.