Skip to content
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.

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

p4Info task throws System.NullReferenceException - BUG #29

Open
bwojtowi opened this issue May 29, 2013 · 5 comments
Open

p4Info task throws System.NullReferenceException - BUG #29

bwojtowi opened this issue May 29, 2013 · 5 comments

Comments

@bwojtowi
Copy link

p4Info executes ToString() on the values Perforce.GetP4Info(find) returns without checking if ithey are not nulls. Taking into account that Perforce.GetP4Info(find) returns an array of string it's not needed at all and introduces a bug only.

protected override void ExecuteTask()
{
string[] find = {"User name:", "Client name:", "Client host:", "Client root:"};
string[] results = Perforce.GetP4Info(find);

Project.Properties[User] = results[0].ToString();            <-- BUG
Project.Properties[Client] = results[1].ToString();           <-- BUG 
Project.Properties[Host] = results[2].ToString();            <-- BUG
Project.Properties[Root] = results[3].ToString();            <-- BUG

}

@dguder
Copy link
Member

dguder commented Jun 3, 2013

Hi, thanks for the hint. What's your proposal now?
Some thoughts:
What caused the error to show up right now? Was anything changed by perforce, so this error can happen? Maybe the command line output which is used to find the entries by the search patterns: "User name:", "Client name:", "Client host:", "Client root:" ? Unfortunately I don't have access to perforce so I can't verify this.

@bwojtowi
Copy link
Author

bwojtowi commented Jun 4, 2013

There might be several reasons for this. However, I suspect one of the properties is null in the context I'm running the p4 command.
Amending the code to the below would be the first thing I’d try.

protected override void ExecuteTask() 
{

string[] find = {"User name:", "Client name:", "Client host:", "Client root:"};

string[] results = Perforce.GetP4Info(find);
if(results == null) throw new Exception(Failed to retrieve P4 info.);
Project.Properties[User] = results.Length > 0 ? results[0] :unknown;
Project.Properties[Client] = results.Length > 1 ? results[1] :unknown;
Project.Properties[Host] = results.Length > 2 ? results[2] :unknown;
Project.Properties[Root] = results.Length > 3 ? results[3] :unknown;
}

@bwojtowi
Copy link
Author

bwojtowi commented Jun 4, 2013

Also, It might be useful to add a parameterized task GetP4Info for retrieving individual info fields.

@dguder
Copy link
Member

dguder commented Jun 4, 2013

Hi,
hmm.... IMHO this length check won't help since GetP4Info will always return a string array with length of find.Length. In addition the result will not be null either.

Could you please show me your output of your p4 info when you run it on your command line? As stated at p4 info docs the current implementation is still valid. Are you using a localized version?

@bwojtowi
Copy link
Author

bwojtowi commented Jun 5, 2013

Assuming the “results” array is not null, not calling ToString() on each element would help in the sense it would retrieve the maximum information available instead of throwing a NullReferenceException. It's a good practice to implement validation in your code instead of relying on what thirdparty developers say. This issue itself shows why the best. That is the reason for additional checks to the length and results not being null. You never know in what context people may run it in the future.

I use standard version I believe.

Here’s the output my p4 info cmd line (I changed the environment specific values). It seems the Client Root is not there for some reason.

C:\Documents and Settings\user1>p4 info
User name: user1
Client name: currentbox
Client host: currentbox
Client unknown.
Current directory: c:\Documents and Settings\user1
Peer address: localIPAddress:clientport
Client address: localIPAddress
Server address: serverbox:serverport
Server root: /perforce/data
Server date: 2013/06/05 11:22:08 +0100 BST
Server uptime: 248:59:20
Server version: P4D/LINUX26X86_64/2012.2/538478 (2012/10/16)
Server license: licence details
Server license-ip: serverbox
Case Handling: insensitive

C:\Documents and Settings\user1>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants