dotnet-r2pipe on Windows just hangs

[hackathi]

dotnet-r2pipe on Mono 3.12 using radare2 0.9.9-git aka radare-0.9.8-813.g37cfb20 commit 7161 works fine and as expected.

The same code under Windows however doesn't run. radare2 0.9.9-git aka jenkins-radare2-w32-4239 commit 7779 just hangs and uses somewhere between 10 and 20% CPU constantly.

Tested with both master dotnet-r2pipe and my reworked fork. Same behavior.

[radare]

Should we close this issue after your rewrite? i found several issues that must be addressed in the new code:

• make fails to run because main.cs is not using the new api
• the used is forced to pass the executable path, where it must be checked in $PATH by default
• not sure if async api is still usable (OLDNETFX)
• there's no way to open the file in read-write (-w) or without loading the file header (-n) like other bindings do.
• what about using a single constructor that instantiates a different method depending on the given url? like the rest of bindings do?

[radare]

Also, do not use windows-style newlines. Just run dos2unix or perl -ne 's/\r\n/\n/g;print'

[hackathi]

Well, the crash still happens regardless of the binding version, so this issue shoud still be open. IRC directed me to open thi issue here, altough I'm not sure if it shouldn't belong to the radare repo.

Regarding your other points:

• Yeah, th Makefile needs work. Sorry about that.
• You need to pass the file you want to examine. You optionally can give a path to the radare2 binary, which was hardcoded before. What would suggested behavior be?
• Options can be added, that's no problem. Just a few constructors.
• I personally oppose a single constructor which handles everything, as it violates the single responsibility paradigm IMO. One could however do a factory for these which would autodetect the class to use.

Regarding Line Endings: I fixed my git and it should commit only Unix-Style Endings now.

[radare]

replies: - Fixing the makefile implies fixing the the main.cs - Feels strange to specify the r2 path in the constructor, other bindings just accept options as argument, maybe we can just define an object that specifies r2 path, load options (-n, -w) and pass it as argument to the constructor. - As long as loading options must be passed on constructor and not after, we can just follow this pattern (pass an instance with user settings) - Ok about the single constructor thing, r2pipe bindings aims to be more idiomatic. I was using main.cs as a 'testsuite', as a way to ensure nothing was broke. Maybe we can add some unit tests using Nunit or whatever it feels more native to .net

[radare]

Also, im a bit concerned about the dependencies of this r2pipe.dll. it would be good if this lib can be used on windows phone, where the spawn apis are forbidden/unexistant.

What about the async api?

Also, I miss some native JSON parsing, in the sense that cmdj() parses the json of the output of the command and returns an associated object that can be walked by the user.

[radare]

in the python bindings i have also added syscmd() and syscmdj() commands to run commandline programs and get its output (or parse json). useful for spawning r*2.exe commands

[hackathi]

Regarding main.cs: It's completely outdated. I have implemented a successor here.

Feels strange to specify the r2 path in the constructor, other bindings just accept options as argument, maybe we can just define an object that specifies r2 path, load options (-n, -w) and pass it as argument to the constructor.

Well, that would be possible. But I think adding a few constructors with another string argument would be simpler. The problem with not having a possibility to pass the r2 executable in the constructor is Windows (that would affect the other bindings too, I'm afraid) or generally having multiple r2 versions installed (think of one in your PATH and one in, let's say, /opt).

Ok about the single constructor thing, r2pipe bindings aims to be more idiomatic.

As I said, Implementing a factory is no problem and you have one single method which can instanciate everything :) The Problem is that whole dotnet has special classes for everything which implement an interface or inherit from other super classes (think Streams for example: everything is derived from Stream, but to use it you have to use MemoryStream, FileStream, ...). The Interface approach also cleans up the code a lot IMO.

I could port this to Windows Phone, but:

• A separate project file would be required (altough code can be shared)
• I don't think this would ever compile under *nix
• As far as my knowledge of the Windows RT APIs is, there are only the methods with the async keyword availiable. As far as the API is implemented, I think only the HTTP r2pipe would really work, with the tcp r2pipe which could work if it would be implemented.

As for async... To get real async in terms of parallelism I've implemented async methods, however you need an async context to use it. For async in terms of callback, there is the queued r2pipe.

The OLDNETFX compiler variable just excludes all methods with async keywords, because the async keyword was only intruduced with dotnet 4.0. You wouldn't be able to build this with older mono / dotnet versions otherwise.

As for JSON... dotnet lacks JSON parsing in the standard classes. However, there is newtonsoft json, which is a popular library licensed under MIT. If I were to add deserialization support to r2pipe, I'd have to get the nuget package for it and that would also mean you need internet access to build this (you'd need to fetch the package for linking). Also you need nuget.exe somewhere if you were to build it outside of monodevelop / visual studio.

Edit: Unit tests would be cool, tough.

[radare]

What about https://msdn.microsoft.com/en-us/library/system.json%28v=vs.95%29.aspx ? it's not really clear which versions include support for it. Besides being a quite basic stuff nowadays.

Adding more parameters to the constructors will have some problems:

• adding new options will make constructor longer
• will require/break the rest of classes that must have compatible usage

My suggestion is having R2PipeOptions class which can be configured as follows:

var opts = new R2PipeOptions();
opts.path = @"c:\radare2\radare2.exe";
var r2p = new R2Pipe("rax2.exe", opts);
...

opts must be optional

[hackathi]

Yeah, if you'd like one could do the R2PipeOptions - altough the huge constructor only gets in your way if you insist on the factory, which is the exact opposite of how the whole rest of dotnet works. I won't argue on this, it's based mostly on personal believes, and as long as using this factory is optional, I'm game. I personally think the language bindings should orientate on how the rest of the language's framework is working rather than how the rest of the bindings work. You can't compare dotnet to haskell or even c. Same goes for naming conventions. But again, I won't argue with that as long as this equality-layer is optional.

C# knows constructor overloading. Adding more optional parameters is just a line of sane constructor chaining. If a mandatory parameter was to be added, it would be visible at compile time because of the signature change that would generate a compiler error. In my opinion, this isn't neccessarily a bad thing - you could find errors at compile time which would otherwise only be visible at runtime and thus save a few hours debugging.

Another Option which might be interesting looking at are optional named arguments, altough I'm not a too big fan of them, as they tend to increase the code wtfs per minute (tm).

The JSON Namespace you linked only exists in the deprecated silverlight (microsofts attempt to push flash out of the market) assemblies. They certainly don't work on Windows Phone or the dot net runtime, and I wouldn't include them in new stuff either. dotnet once had a core json engine, but it was outsourced to a nuget package and is now unlisted, which means it's deprecated and shouldn't be used. In fact, the visual studio templates for asp.net json related stuff automatically install newtonsoft json for you - I think that's a clear sign from Microsoft.

[radare]

this is fixed now