This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Process record and replay, 5/10
> Date: Sun, 9 Nov 2008 11:53:54 +0800
> From: teawater <teawater@gmail.com>
> Cc: gdb-patches@sourceware.org
>
> >> +/* These macros are the values of the first argument of system call
> >> + "sys_ptrace". The values of these macros are gotten from Linux Kernel
> >> + source. */
> >> +
> >> +#define RECORD_PTRACE_PEEKTEXT 1
> >> +#define RECORD_PTRACE_PEEKDATA 2
> >> +#define RECORD_PTRACE_PEEKUSR 3
> >
> > Again, shouldn't this kind of data be taken from the syscall database,
> > rather than being spread over a few source files? I think having them
> > in one place will make the code more maintainable.
>
> What about I make a special .h file for each of them?
I was referring to the syscall database that is part of the "catch
syscalls" patch discussed elsewhere in this list. I think we should
have all the data about Linux system calls in the same place.
> >> + /* sys_ni_syscall */
> >> + case 56:
> >> + /* sys_setpgid */
> >> + case 57:
> >> + /* sys_ni_syscall */
> >> + case 58:
> >> + break;
> >> +
> >> + /* sys_olduname */
> >> + case 59:
> >> + regcache_raw_read (record_regcache, tdep->arg1, (gdb_byte *) & tmpu32);
> >> + if (record_arch_list_add_mem (tmpu32, tdep->size_oldold_utsname))
> >> + {
> >> + return (-1);
> >> + }
> >> + break;
> >> +
> >> + /* sys_umask */
> >> + case 60:
> >> + /* sys_chroot */
> >> + case 61:
> >> + break;
> >> +
> >> + /* sys_ustat */
> >> + case 62:
> >> + regcache_raw_read (record_regcache, tdep->arg2, (gdb_byte *) & tmpu32);
> >> + if (record_arch_list_add_mem (tmpu32, tdep->size_ustat))
> >> + {
> >> + return (-1);
> >> + }
> >> + break;
> >
> > It's a matter of style, I guess, but wouldn't it be better, instead of
> > endless repetition of almost identical code fragments like the two
> > above, to put the differing chunks in some data structure and then
> > just have one instance of the call to regcache_raw_read and
> > record_arch_list_add_mem, using the data in the data structure?
> >
>
> Sorry, I am not very clear your mean. I am not native speaker. :(
>
> Do you mean is put this code to a function that has a argv is tdep?
No, I mean to define a data structure, like this:
struct syscall_entry {
int num;
size_t size;
} syscall_data[] = {
...
{ 56, 0 },
{ 57, 0 },
{ 58, 0 },
{ 59, tdep->size_oldold_utsname },
...
};
and then use it like this:
if (syscall_data[i].size)
{
regcache_raw_read (record_regcache, tdep->arg1, (gdb_byte *) & tmpu32);
if (record_arch_list_add_mem (tmpu32, syscall_data[i].size))
{
return (-1);
}
}
break;
You can then have only one (or maybe few different) code fragments,
and the rest will be recorded in the data structure.
> >> + case RECORD_SYS_GETPEERNAME:
> >> + {
> >> + uint32_t a[3];
> >> + regcache_raw_read (record_regcache, tdep->arg2,
> >> + (gdb_byte *) & tmpu32);
> >> + if (tmpu32)
> >> + {
> >> + if (target_read_memory (tmpu32, (gdb_byte *) a, sizeof (a)))
> >> + {
> >> + fprintf_unfiltered (gdb_stdlog,
> >> + "Record: read memory addr = 0x%s len = %d error.\n",
> >
> > Is this a left-over from debugging stage? If not, why is it needed in
> > GDB? (There are few more fprintf_unfiltered's like this one.)
> >
> I want let user know what happen when got a error. What do you think about it?
I think we should at least have a user option to turn it on and off.