This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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?



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]