Re: [PATCH] Makefile.in includes linux-record.c to be common for all arch. (arm-reversible>phase-3)

[Hui Zhu <teawater@gmail.com>]

On Tue, Jun 19, 2012 at 2:49 AM, oza Pawandeep <oza.pawandeep@gmail.com> wrote:
> Hi Hui,
>
> The phase2 works indepedently. It does not need syscall really. If i recall
> correctly michael snyder suggested that i make two patches.  The first patch
> contains arm instructions and the and second part contains linux abi
> support.

I think he's means is divide the patch to insn part and syscall part.
But if you want to post to maillist or commit to cvs tree.  I think
they need to be commit together.
The reason is without the syscall-record support, how the patch test
with the testsuite?  Without that, How do you prove that your code is
correct?
For the x86-record code, the insn and syscall patch is commit
together.  So go back to my suggest, move all the code about arm
record to a separate branch.  And when you done all of them and past
the test, re-commit them.

>
> The second part which i am working now requires linux-record.o hence i wrote
> we require it to be compiled with the second part of patch.
>
> So first i try to chek in minor change of congpfigure.tgt
> And then i check syscall record on arm.
>
> By the way there is one more query which has been there under discussion.
> When you made gdb sys call defination, it was thought as generic, but it
> does not turn out to be applicable for arm as syscall number differs.
> Sometime back tom had suggestion of having xml files under gdb/syscalls  for
> arm arch and x86 separately; do you have any inputs to it?  Of course it
> would change x86 syscall record to be read from xml files.j

Do you really see the code of syscall-record part?  I suggest you
re-read the code.
The linux-syscall-record code can be work with most of the arch
because before call record_linux_system_call, the syscall number will
be translate to enum gdb_syscall.  You can see the
amd64_canonicalize_syscall as the example.

Thanks,
Hui

>
> Regards,
> Oza.
>
> On Jun 18, 2012 2:22 PM, "Hui Zhu" <teawater@gmail.com> wrote:
>>
>> On Mon, Jun 18, 2012 at 7:49 PM, oza Pawandeep <oza.pawandeep@gmail.com>
>> wrote:
>> > Yes I agree; as I integrated both of them and post them at once.
>> > sorry about confusion; this patch has to be ignored.
>> >
>> > In fact I wanted this patch to be approved first because without which
>> > sys call patch would not compile.
>>
>>
>> Why you cannot commit a patch list when the function is done?
>> I think the function in the trunk tree need be done before commit to
>> it.  If you want work in cvs, I suggest you use the branch first.
>>
>> On the other hand, I heard that some of code of arm record is checked
>> in.  I don't think it is right.  Because without syscall support, it
>> cannot work, right?
>> So what I suggest is move all the code about arm record to a separate
>> branch.  And when all of the arm record function done, you re-send all
>> of them.
>>
>> Thanks,
>> Hui
>>
>>
>>
>> >
>> > Regards,
>> > Oza.
>> >
>> > On Mon, Jun 18, 2012 at 2:54 PM, Yao Qi <yao@codesourcery.com> wrote:
>> >> On 06/18/2012 05:08 PM, oza Pawandeep wrote:
>> >>> diff -urN orig/configure.tgt new/configure.tgt
>> >>> --- orig/configure.tgt        2012-06-18 12:36:47.274501400 +0530
>> >>> +++ new/configure.tgt 2012-06-18 12:31:47.335501400 +0530
>> >>> @@ -76,7 +76,7 @@
>> >>>  arm*-*-linux*)
>> >>>       # Target: ARM based machine running GNU/Linux
>> >>>       gdb_target_obs="arm-tdep.o arm-linux-tdep.o glibc-tdep.o \
>> >>> -                     solib-svr4.o symfile-mem.o linux-tdep.o"
>> >>> +                     solib-svr4.o symfile-mem.o linux-tdep.o
>> >>> linux-record.o"
>> >>>       build_gdbserver=yes
>> >>>       ;;
>> >>>  arm*-*-netbsd* | arm*-*-knetbsd*-gnu)
>> >>>
>> >>> ok to check in ?
>> >>
>> >> It is not good to post the same change twice in different mails.  This
>> >> change makes no sense until your 'arm-syscall record' patch is
>> >> approved.
>> >>  I noticed that this change has been included in your 'arm-syscall
>> >> record' patch, so I think patch here doesn't have to reviewed.
>> >>
>> >> --
>> >> Yao (齐尧)
>> >>
>> >>