-
-
Notifications
You must be signed in to change notification settings - Fork 133
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
License #87
Comments
How does it related to our project? since you are using Robert Giesecke's code and in the fact that Robert Giesecke didn't mention that his code was based on my code(see the il parser code) which was under CPOL license which is not compatible with MIT anyway, the only point that i wrote this is the point 5a and 5e of the license "You agree not to remove any of the original copyright" which was "Copyright © Osadkowski S.A. 2006" |
Any idea @RobertGiesecke? |
anyway I've change license of article to MIT example of code article
version 1.2.1.28776 DllExport
|
Hmm, I've checked both of your direction in logic for parsing the IL code, so partially it seems to be true. It more like he was at least inspired by the your idea of your parser. But, the current source code (this repo continues 1.2.7), has a very weak relationship with your code, for example: You have this adding: if (trimedline.StartsWith(".corflags"))
{
wholeilfile.Add(".corflags 0x00000002");
wholeilfile.Add(string.Format(".vtfixup [{0}] int32 fromunmanaged at VT_01", exportscount));
wholeilfile.Add(string.Format(".data VT_01 = int32[{0}]", exportscount));
Console.WriteLine("Adding vtfixup.");
addilne = false;
} It seems it was necessary to explicitly set the runtime header flags in ILAsm v1.x (an manual adding .corflags + .vtfixup -> .data) but our IL parser works only with 2.0+ So we have no this at all. While the switch (state)
{
case ParserState.Normal:
if (trimedline.StartsWith(".corflags"))
{
...
addilne = false;
}
else if (trimedline.StartsWith(".class"))
{
...
addilne = false;
}
else if (trimedline.StartsWith(".assembly extern ExportDllAttribute"))
{
addilne = false;
...
} Robert's logic looks like: public override void Execute(ParserStateValues state, string trimmedLine)
{
if(trimmedLine.StartsWith(".class", StringComparison.Ordinal))
{
...
state.AddLine = true;
}
else if(trimmedLine.StartsWith(".method", StringComparison.Ordinal))
{
...
state.AddLine = false;
}
....
public override void Execute(ParserStateValues state, string trimmedLine)
{
if(trimmedLine.StartsWith(".corflags", StringComparison.Ordinal))
{
...
state.AddLine = false;
}
else if(trimmedLine.StartsWith(".class", StringComparison.Ordinal))
{
...
state.AddLine = true;
} By the way, looks like you will have a similar to #59 bug for: case ParserState.Method:
if (trimedline.StartsWith("} // end of method")) Even your parsing in ParserState.MethodDeclaration is not the same at all. You have trivial regex Your mentioned ClassDeclarationParserAction from v1.2.7 contains own iteration. But for stack-based logic it's normal to push found elements before new deepening. The only one part from 1.2.7 that really looks like this is you, the following ParserState enum: enum ParserState
{
Normal,
ClassDeclaration,
Class,
DeleteExportDependency,
MethodDeclaration,
MethodProperties,
Method,
DeleteExportAttribute,
} So I need to hear Robert's story, or I need detailed code matching. Please continue to compare 1.2.7 version, because more probably Robert was trying to avoid dependencies with you if it was (seems it was) in earlier versions. Thus, he could completely rewrite initial logic. Or he was just inspired by your idea. I don't know. But anyway we don't follow any versions less than 1.2.7. Mainly I agree, the parsing logic looks similar in direction like you but StartsWith/addLine/stack-based iteration is a common way for understanding. And most important, we do not set the runtime header flags at all, not even .vtfixup, and also methods parsing is not similar at all. tl;dr I'm not against to add you in Licence, but I don't know the whole story about this. Please understand, I'll try to solve your issue soon as possible. Briefly, as I explained above, I can confirm only ParserState enum without logic for 1.2.7. So please everyone, clarify this as possible. I'll try to contact directly with Robert if he doesn't answer here. |
Hi, I remember that some things were eerily similar. I also used a state machine and my enum was almost the same. However, if I remember correctly, I already used Cecil in my first prototypes to get the attributes and their values reliably. I am absolutely unsure (and curse myself for not using a VCS back then), whether I followed your approach to get rid of the attribute dependency or not. The obvious pieces like naming a variable „trimmedLine“ are no indication for a copy, I used that name so many a hundred times for a trimmed line, as did probably everybody else. And btw, this repo here is not my code. He used ilspy or reflector to get C# code from my binaries. |
Fair enough, please close the issue |
:) lot of things. There was many products from 2015-2017 with breaking of backward compatibility for paths and features in that period. Also some hard-coded moments like when I was starting distribute ILAsm on coreclr for this project that required external converter of resources .res -> obj COFF-format (to place in .rsrc section - PE). So now there were a number of my new private/open things and many discussions with MS to avoid these incomprehensible actions and <_<
Okay, it seems the issue has been resolved. Closed. |
In original code there is lack of license to original code (published in 2006 with CPOL license)
You can see that that was a base code in state's names and state's values like
addilne
,classnames
The text was updated successfully, but these errors were encountered: