[Click] Minimizing patch size

[Philip Prindeville]

Inline...


On 1/4/11 11:18 AM, Joonwoo Park wrote:
> Hi Philip,
>
> (2) The reason of patching "::" to ": :" is because "::" is namespace
> keyword of c++ as you know.
> It must be true that we can leave "::" in .c file as we builds .c with
> c compiler not c++, however if "::" is in .h, since that .h will
> possibly be included from .cpp file click linuxmodule, we need to
> patch "::" to ": :" by all means.

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.


> Therefore I made a simple script to change every "::" to ": :" and
> generated patch when I made patch for 2.6.24.7, I think you can revert
> "::" changes in .c files if you'd like to do.
> (3) compat shim would seem to me for finding out correct symbol table,
> but since click uses c++ compiler .h has be modified to make c++
> compiler happy,  I think we still need to provide c++ compile-able .h
> files.  ie) structure member initializer, c++ keyword and etc... which
> cannot be solved by 'extern "C"' keyword.

Have you tried?  What was the result?

C is supposed to be a subset of the C++ language when in the "C" namespace, which is activated by extern "C".

> (1) At any rate, for recent version of linux, I believe right
> direction is improving patchless to support newer version without
> patching linux.
>
> Joonwoo
>
> On Mon, Jan 3, 2011 at 11:54 PM, Philip Prindeville
> <philipp_subx at redfish-solutions.com>  wrote:
>> I was looking at the click kernel patches and noticed two things.
>>
>> (1) none of the kernels seem to be particularly recent.
>> (2) a lot of the patches seem to be something other than added functionality.
>>
>> Looking, for instance, at:
>>
>> diff --git a/arch/arm/mach-iop13xx/irq.c b/arch/arm/mach-iop13xx/irq.c
>> index 69f07b2..e217167 100644
>> --- a/arch/arm/mach-iop13xx/irq.c
>> +++ b/arch/arm/mach-iop13xx/irq.c
>> @@ -38,7 +38,7 @@ static u32 read_intctl_0(void)
>>   }
>>   static void write_intctl_0(u32 val)
>>   {
>> -       asm volatile("mcr p6, 0, %0, c0, c4, 0"::"r" (val));
>> +       asm volatile("mcr p6, 0, %0, c0, c4, 0": :"r" (val));
>>   }
>>
>> ...
>>
>>
>> I'm wondering why this is required? First, why would the C++ compiler try to compile a .c file, and second, why would it not be able to parse an asm() directive correctly?
>>
>> Looking over the patches, the "essential" ones are less than a thousand lines.
>>
>> (3) I work a bit with the compat-wireless group, and they've managed to come up with a loadable module that provides "shim" functionality that allows the most recent wireless (wifi) drivers to load and run against older kernels.  I can use a 2.6.37 driver with a 2.6.27 kernel (even with the skb changes that occurred in the interim).
>>
>> It should be possible to do something similar for click.
>>
>> The only substantive change is adding new member fields to netdevice.h.
>>
>> What about using a Makefile adaptation like:
>>
>> %.o: %.c:
>>         (echo '#ifdef __cplusplus' ; \
>>          echo 'extern "C" {' ; \
>>          echo '#endif' ; \
>>          echo '# 0 "$<"' ; \
>>          cat $<    ; \
>>          echo '#ifdef __cplusplus' ; \
>>          echo '}' ; \
>>          echo '#endif' ) | $(CC) $(CFLAGS) -c - -o $@
>>
>>
>> to bracket the contents of C files with:
>>
>> #ifdef __cplusplus
>> extern "C" {
>> #endif
>> ...
>> #ifdef __cplusplus
>> }
>> #endif
>>
>> and compile them that way?
>>
>> I'm not sure if the debugging symbols would still be valid, or if they'd point to the wrong source files, but there are ways to patch that up with objcopy.
>>
>> Comments?
>>
>> Thanks,
>>
>> -Philip
>>
>> _______________________________________________
>> click mailing list
>> click at amsterdam.lcs.mit.edu
>> https://amsterdam.lcs.mit.edu/mailman/listinfo/click
>>