Two Gendarme rules for interoperability

GoogleCodeExporter commented 9 years ago

This task is to create 2 specific rules for Gendarme [1] to help
interoperability with native code.

1. PInvokeShouldNotBeVisibleRule

An easy rule to ensure that p/invoke declarations aren't directly visible
outside the assembly.

2. CallGetLastErrorAfterPInvokeRule

A rule that ensure that GetLastError, if used, is called right after the
p/invoke, so that no other calls had the chance to reset the error code.
See [3] for some valid calls between the p/invoke and GetLastError.

To complete the task both rules must include:
- the source code;
- unit tests to ensure the rules works correctly;
- documentation for the web site (see [3] for an example)

This tasks should take between 2-5 days (depending if you already know
Gendarme/Cecil or not).

[1] http://www.mono-project.com/Gendarme
[2]
http://www.gotdotnet.com/Team/FxCop/Docs/Rules/Interoperability/CallGetLastError
ImmediatelyAfterPInvoke.html
[3] http://groups.google.com/group/gendarme

Original issue reported on code.google.com by sebastie...@gmail.com on 27 Dec 2007 at 3:22

GoogleCodeExporter commented 9 years ago

I claim this task.

Original comment by andreas....@gmail.com on 29 Dec 2007 at 2:35

GoogleCodeExporter commented 9 years ago

go! :)

Original comment by sebastie...@gmail.com on 29 Dec 2007 at 2:39

Changed state: Claimed
Added labels: ClaimedBy-andreas.noever, DueDate-2008-01-06

GoogleCodeExporter commented 9 years ago

Part one :)

Original comment by andreas....@gmail.com on 29 Dec 2007 at 4:23

Attachments:

GoogleCodeExporter commented 9 years ago

Part two.

Original comment by andreas....@gmail.com on 29 Dec 2007 at 6:06

Attachments:

GoogleCodeExporter commented 9 years ago

Sorry I'm short on time today (and will be out of town for the next two days). 
I'll
try to review more stuff tonight (in about 6 hours). For the moment...

I think you're missing calls to object creation in the second rule.
E.g.

public void CallPInvoke_CreateObject_GetError ()
{
    MessageBeep(5);
    new CallGetLastErrorAfterPInvokeTest ();
    Marshal.GetLastWin32Error();
}

since the rule has no clue what's being done in the ctor it should warn that 
the call
to GetLastError is too late.

Second note: I forgot to say (but it's no big deal ;-) but the two rules will 
be in a
new assembly (and namespace) named Gendarme.Rules.Interoperability

Original comment by sebastie...@gmail.com on 30 Dec 2007 at 8:20

GoogleCodeExporter commented 9 years ago

Yes, object creation was missing. I have added newobj. Initobj is not needed, 
since
it just initializes the memory for the struct. (Parameterized struct 
constructors are
invoked with call. I have added a test for this).

The namespace is now: Gendarme.Rules.Interoperability

Documentation:
Gendarme.Rules.Interoperability

PInvokeShouldNotBeVisibleRule
This rule checks for PInvoke methods that are internal to the analyzed assembly.

Bad exmaple:
[DllImport ("User32.dll")]
public static extern Boolean MessageBeep (UInt32 beepType);

Good example:

[DllImport ("User32.dll")]
internal static extern Boolean MessageBeep (UInt32 beepType);

CallGetLastErrorAfterPInvokeRule
Marshal.GetLastWin32Error () should be called directly after a PInvoke call.
Intermediate calls could overwrite the error.

Bad example:
public void DestroyError () {
    MessageBeep (2);
    Console.WriteLine ("Beep");
    int error = Marshal.GetLastWin32Error ();
}

Good example:
public void GetError () {
    MessageBeep (2);
    int error = Marshal.GetLastWin32Error ();
    Console.WriteLine ("Beep");
}

public void DontUseGetLastError () {
    MessageBeep (2);
    Console.WriteLine ("Beep");
}

Original comment by andreas....@gmail.com on 30 Dec 2007 at 9:21

Attachments:

GoogleCodeExporter commented 9 years ago

got something important missing in #2 ... ;-)

Original comment by sebastie...@gmail.com on 31 Dec 2007 at 12:31

GoogleCodeExporter commented 9 years ago

fixed :) ( + unittest )

Original comment by andreas....@gmail.com on 31 Dec 2007 at 12:47

Attachments:

GoogleCodeExporter commented 9 years ago

Ok, everything is fine and complete :)
Closing!

p.s. It's fun to see it doesn't get repetitive ;-)

Original comment by sebastie...@gmail.com on 31 Dec 2007 at 1:10

Changed state: Closed

GoogleCodeExporter commented 9 years ago

As discussed in IRC here is an update that supports branching (simple version 
of #82,
does not support try/catch etc, just simple branches and switches). It also 
needs
access to AddIfNew and StackEntryAnalysis.GetNextInstruction (needs to be public
static) from #82.

The rule follows all branches and triggers if one branch calls a (not 
whitelisted)
method and GetError afterwards.

Original comment by andreas....@gmail.com on 26 Jan 2008 at 1:45

Attachments:

GoogleCodeExporter commented 9 years ago

Updated to fix the multiple PInvoke bug.

Original comment by andreas....@gmail.com on 30 Jan 2008 at 4:16

Attachments:

GoogleCodeExporter commented 9 years ago

The two last defects are real one, so no more false positives!
Thanks a lot for continuing the fixes after the task was closed :)
Sebastien

Original comment by sebastie...@gmail.com on 30 Jan 2008 at 4:38

shana / google-highly-open-participation-mono

Two Gendarme rules for interoperability #63