Re: [RFA/RFC Prec] Add Linux AMD64 process record support second version, (AMD64 Linux system call support) 3/3

Hui Zhu
2009-07-19T08:21:17Z

On Sat, Jul 18, 2009 at 09:14, Michael Snyder<msnyder@vmware.com> wrote:
> Hui Zhu wrote:
>
>
>> +  /* Convert tmpulongest to number in record_linux_system_call.  */
>> +  switch (tmpulongest)
>> +    {
>> +      /* sys_read */
>> +    case 0:
>> +      num = 3;
>> +      break;
>> +      /* sys_write */
>> +    case 1:
>> +      num = 4;
>> +      break;
>
> Hey Hui,
>
> This switch statement is over 1000 lines long!  ;-)
>
> It's OK, there's no real rule against that, but it just
> makes me think about whether shortening it might make it
> any easier to read and maintain...
>
> I thought of suggesting a look-up table, but that would
> actually make it harder to read and maintain, I think...
>
> What about this?  If you wrote it this way...
>
>        case 1:         /* sys_write */
>
> you'd save over 250 lines, and I think it would be more readable.
>
> And then, if you abstracted the switch statement out
> into a separate function, you could code it like this...
>
>        case 1:         /* sys_write */
>          return 4;
>        case 2:         /* sys_open */
>
> and save another 250 lines, cutting the whole thing by half.
> You'd have to special-case number 158, of course.
>
> I leave it up to you, you can decide.
>
> Other than that it looks fine.  Mark?
>
>
>

That is really a big work. Please let me post a special patch for it later.

Thanks,
Hui