Skip to content

Let's (try to) make Coverity's job easier #1090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 11, 2015
Merged

Let's (try to) make Coverity's job easier #1090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6747421
Prevent multiple enumerations
nulltoken Jun 11, 2015
e4bb412
Fix build warnings by not relying on obsolete properties
nulltoken Jun 11, 2015
8212155
Remove duplicated code
nulltoken Jun 11, 2015
02264de
Simplify GitObjectIsNotNull implementation
nulltoken Jun 11, 2015
611e412
Link to Coverity from the README
nulltoken Jun 11, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion LibGit2Sharp.Tests/FilterFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ private FileInfo CheckoutFileForSmudge(string repoPath, string branchName, strin
private static FileInfo CommitFileOnBranch(Repository repo, string branchName, String content)
{
var branch = repo.CreateBranch(branchName);
repo.Checkout(branch.Name);
repo.Checkout(branch.FriendlyName);

FileInfo expectedPath = StageNewFile(repo, content);
repo.Commit("Commit");
Expand Down
6 changes: 3 additions & 3 deletions LibGit2Sharp.Tests/FilterSubstitutionCipherFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void SmugdeIsNotCalledForFileWhichDoesNotMatchAnAttributeEntry()
Assert.Equal(0, filter.SmudgeCalledCount);

var branch = repo.CreateBranch("delete-files");
repo.Checkout(branch.Name);
repo.Checkout(branch.FriendlyName);

DeleteFile(repo, fileName);

Expand Down Expand Up @@ -75,7 +75,7 @@ public void CorrectlyEncodesAndDecodesInput()
Assert.Equal(0, filter.SmudgeCalledCount);

var branch = repo.CreateBranch("delete-files");
repo.Checkout(branch.Name);
repo.Checkout(branch.FriendlyName);

DeleteFile(repo, fileName);

Expand Down Expand Up @@ -181,7 +181,7 @@ public void SmudgeIsCalledIfAttributeEntryMatches(string filterAttribute, string
CommitOnBranchAndReturnDatabaseBlob(repo, fileName, decodedInput);

var branch = repo.CreateBranch("delete-files");
repo.Checkout(branch.Name);
repo.Checkout(branch.FriendlyName);

DeleteFile(repo, fileName);

Expand Down
39 changes: 8 additions & 31 deletions LibGit2Sharp/Core/Ensure.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,44 +250,21 @@ public static void ArgumentPositiveInt32(long argumentValue, string argumentName
/// <param name="identifier">The <see cref="GitObject"/> identifier to examine.</param>
public static void GitObjectIsNotNull(GitObject gitObject, string identifier)
{
Func<string, LibGit2SharpException> exceptionBuilder;

if (string.Equals("HEAD", identifier, StringComparison.Ordinal))
{
exceptionBuilder = m => new UnbornBranchException(m);
}
else
if (gitObject != null)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been meaning to ask about this: is checking null all that needs to be done here or should some SafeHandle or IntPtr be checked as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please open an issue about this?

{
exceptionBuilder = m => new NotFoundException(m);
return;
}

GitObjectIsNotNull(gitObject, identifier, exceptionBuilder);
}

var message = string.Format(CultureInfo.InvariantCulture,
"No valid git object identified by '{0}' exists in the repository.",
identifier);

/// <summary>
/// Check that the result of a C call that returns a non-null GitObject
/// using the default exception builder.
/// <para>
/// The native function is expected to return a valid object value.
/// </para>
/// </summary>
/// <param name="gitObject">The <see cref="GitObject"/> to examine.</param>
/// <param name="identifier">The <see cref="GitObject"/> identifier to examine.</param>
/// <param name="exceptionBuilder">The builder which constructs an <see cref="LibGit2SharpException"/> from a message.</param>
public static void GitObjectIsNotNull(
GitObject gitObject,
string identifier,
Func<string, LibGit2SharpException> exceptionBuilder)
{
if (gitObject != null)
if (string.Equals("HEAD", identifier, StringComparison.Ordinal))
{
return;
throw new UnbornBranchException(message);
}

throw exceptionBuilder(string.Format(CultureInfo.InvariantCulture,
"No valid git object identified by '{0}' exists in the repository.",
identifier));
throw new NotFoundException(message);
}
}
}
7 changes: 5 additions & 2 deletions LibGit2Sharp/Repository.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1006,14 +1006,17 @@ public void CheckoutPaths(string committishOrBranchSpec, IEnumerable<string> pat
Ensure.ArgumentNotNullOrEmptyString(committishOrBranchSpec, "committishOrBranchSpec");
Ensure.ArgumentNotNull(paths, "paths");

var listOfPaths = paths.ToList();

// If there are no paths, then there is nothing to do.
if (!paths.Any())
if (listOfPaths.Count == 0)
{
return;
}

Commit commit = LookupCommit(committishOrBranchSpec);
CheckoutTree(commit.Tree, paths.ToList(), checkoutOptions ?? new CheckoutOptions());

CheckoutTree(commit.Tree, listOfPaths, checkoutOptions ?? new CheckoutOptions());
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion LibGit2Sharp/RepositoryExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ private static Commit RetrieveHeadCommit(IRepository repository)
{
Commit commit = repository.Head.Tip;

Ensure.GitObjectIsNotNull(commit, "HEAD", m => new UnbornBranchException(m));
Ensure.GitObjectIsNotNull(commit, "HEAD");

return commit;
}
Expand Down
14 changes: 13 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,25 @@
[twitter]: http://twitter.com/libgit2sharp

## Current project build status

The CI builds are generously hosted and run on the [Travis][travis] and [AppVeyor][appveyor] infrastructures.

| | Windows (x86/amd64) | Linux/Mac OS X |
| :------ | :------: | :------: |
| **master** | [![master win][master-win-badge]][master-win] | [![master nix][master-nix-badge]][master-nix] |
| **vNext** | [![vNext win][vNext-win-badge]][vNext-win] | [![vNext nix][vNext-nix-badge]][vNext-nix] |

The security oriented static code analysis is kindly run through the [Coverity][coverity] service.

| | Analysis result |
|-------|-----------------|
| **vNext** | [![coverity][coverity-badge]][coverity-project] |


[travis]: http://travis-ci.org/
[travis]: https://travis-ci.org/
[appveyor]: http://appveyor.com/
[coverity]: https://scan.coverity.com/

[master-win-badge]: https://ci.appveyor.com/api/projects/status/8qxcoqdo9kp7x2w9/branch/master?svg=true
[master-win]: https://ci.appveyor.com/project/libgit2/libgit2sharp/branch/master
[master-nix-badge]: https://travis-ci.org/libgit2/libgit2sharp.svg?branch=master
Expand All @@ -47,6 +56,9 @@ The CI builds are generously hosted and run on the [Travis][travis] and [AppVeyo
[vNext-nix-badge]: https://travis-ci.org/libgit2/libgit2sharp.svg?branch=vNext
[vNext-nix]: https://travis-ci.org/libgit2/libgit2sharp/branches

[coverity-project]: https://scan.coverity.com/projects/2088
[coverity-badge]: https://scan.coverity.com/projects/2088/badge.svg

## Quick contributing guide

- Fork and clone locally
Expand Down