[Click] Minimizing patch size

[Eddie Kohler]

Philip, just to speak up.

The most important outstanding project for Click is exactly minimizing 
patch size.  Any help is appreciated!

The first step is to correctly understand the problem.

 > I still don't get this.  Even in a .cpp file, you can have
 >
 > c++ stuff
 > ...
 > extern "C" {
 > #include "c-header-with-double-colon-asms.h"
 > }
 > ...
 >
 > and the compiler should be fine with that.
 >
 > If not, then the compiler is broken.

Not quite.  As Joonwoo points out "extern "C" {}" doesn't tell the C++ 
compiler to stop being a C++ compiler.  What is between the braces must 
still be correct C++: no use of C++ keywords as variables, no use of C++ 
operators like ::, and so forth.  This is the main challenge of Click 
patching.  Since Click's kernel module integrates with Linux, it must 
include Linux's header files; and therefore we must change those header 
files to be equivalent, but valid, C++ header files.

Our patches have used a semi-mechanical process to make a bunch of 
syntactic changes (like :: => : : in asm statements), and then as you 
point out include 1000 lines of real changes.

The way forward is "patchless".  We have written a perl script that 
automates the semi-mechanical part.  The script, 
linuxmodule/fixincludes.pl, generates new, C++-includable header files 
as part of the build process.  These header files are equivalent to the 
Linux header files but safe for C++ compilers we care about.

Unfortunately fixincludes.pl is not totally trivial and we have had to 
update it for new kernel releases.  You can look at it to see what it 
does.  Patches welcome.

The goal of the patchless project is to make a version of Click that can 
be loaded into an UNPATCHED Linux kernel.  This would make life a lot 
easier for many of our users.  However, it's not going to achieve great 
performance.  Patchless doesn't support polling, for example.  And it's 
not clear that we can even support total-patchless Click on more recent 
versions of Linux, which have removed some of the hooks on which we relied.

What we HAVE NOT done is separate out the 1000 lines of "real" changes. 
  If you wanted to help with that process it would be lovely.  Joonwoo, 
Cliff, and others have discussed some changes on the list to support 
newer kernels; I owe responses to them too.

Eddie


On 1/4/11 10:58 PM, Philip Prindeville wrote:
> Ok, I just went back and looked at "click/cxxprotect.h" which includes redefinition for C++ keywords like "new" and "virtual".
>
> So has anyone tried to compile Click with minimal patching (i.e. just what is necessary to add the required instrumentation and hooks) with a recent version of gcc (say 4.3.0 or later)?
>
>
>
> On 1/4/11 7:59 PM, Philip Prindeville wrote:
>> Ok, so it seems to me that the solution is twofold:
>>
>> (1) require a reasonably recent version of gcc;
>> (2) In places where the 'new' keyword is used in header files for inlines, etc. do:
>>
>> #define new __new
>> #include<linux/list.h>
>> #undef new
>>
>>
>>
>>
>> On 1/4/11 6:39 PM, Joonwoo Park wrote:
>>> Hi,
>>>
>>> I don't think 'extern "C"' made any difference.  If you were able to
>>> compile your example you should be able to compile even without
>>> 'extern "C"' from your example.
>>> Don't remember exact gcc version but IIRC earlier version of g++
>>> parses '::' as namespace keyword always.  To support old versions,
>>> patching '::' was necessary.
>>>
>>> Joonwoo
>>>
>>> On Tue, Jan 4, 2011 at 5:35 PM, Philip Prindeville
>>> <philipp_subx at redfish-solutions.com>    wrote:
>>>> On 1/4/11 12:25 PM, Joonwoo Park wrote:
>>>>>         1  #include<iostream>
>>>>>         2
>>>>>         3  extern "C"
>>>>>         4  {
>>>>>         5  struct keyword {
>>>>>         6          int new;
>>>>>         7  };
>>>>>         8  }
>>>>>         9
>>>>>        10  int main()
>>>>>        11  {
>>>>>        12          return 0;
>>>>>        13  }
>>>> I took that and modified it as:
>>>>
>>>> #include<iostream>
>>>>
>>>> extern "C"
>>>> {
>>>> #include<sys/types.h>
>>>>
>>>> void load_ldt(int32_t ldt)
>>>> {
>>>>           asm volatile("lldt %0"::"m" (ldt));
>>>> }
>>>> }
>>>>
>>>> int main()
>>>> {
>>>>           return 0;
>>>> }
>>>>
>>>>
>>>> and it compiled fine.
>>>>
>>>> So that solves part of the problem.
>>>>
>>>> -Philip
>>>>
>>>>
>>>>
>>
>
> _______________________________________________
> click mailing list
> click at amsterdam.lcs.mit.edu
> https://amsterdam.lcs.mit.edu/mailman/listinfo/click