[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] GDB stuff updated

From: Vesa Jääskeläinen
Subject: Re: [PATCH] GDB stuff updated
Date: Fri, 16 May 2008 23:17:23 +0300
User-agent: Thunderbird (Windows/20080421)

Hi Lubomir,

Lubomir Rintel wrote:
Updated GDB stub patch, just header text changes, changelog text fix and
stripped of junk.

Also -- solved the mystery of the dl.c fix; for some reason I managed to
generate a reversed diff. It is required to link the gdb module (symbols
defined with .globl in assembler sources, IIRC)

I am not going to attach all the patches here.
All that's needed is here [1]:

Are those copyright years correct or result of copy paste error? Most of the files in this patch are new to GRUB 2 repository.


I am not big fan of fallthru's in switches. They can easily lead to programming errors. As code is rather short and I assume compiler will optimize it anyway, could you just make clean case for STT_NOTYPE without fallthru.


Should those be stored on folder like: contrib/gdb/


Do we want to support other communication mechanism than serial in future for GDB ? I am asking this because it could be prepared a bit with proper interfaces. That would also make code a bit more tidier.

+++ grub2-gdb/gdb/cstub.c       2008-05-07 10:15:52.000000000 +0200

+int (*grub_gdb_getchar) ();

Add void. Perhaps you could make typedef for it and declaring variable as static?

+static int
+hex (ch)
+     char ch;

Let's be a bit more modern...

grub_gdb_getpacket (void):
It may be good idea to refer to documentation stating framing structure or adding comment about frame.

+++ grub2-gdb/gdb/gdb.c 2008-05-07 09:39:11.000000000 +0200

I do not really like this explicit attachment to serial device. Perhaps we should improve serial module and then make gdb to have communication interface.

+++ grub2-gdb/gdb/i386/idt.c    2008-05-07 09:39:11.000000000 +0200

Why not move this as own functional block to:

Also removing trace for gdb :)

I think we need IDT for other features being planned.

+++ grub2-gdb/gdb/i386/machdep.S        2008-05-07 09:39:11.000000000 +0200
We do not do bunnies...

grub_idt_load: Why not move this to startup.S.

+++ grub2-gdb/include/grub/i386/gdb.h   2008-05-07 09:39:11.000000000 +0200

Seperate IDT stuff..

+++ grub2-gdb/include/grub/i386/pc/kernel.h 2008-05-07 09:39:11.000000000 +0200

+#define GRUB_KERNEL_MACHINE_RAW_SIZE           0x4BC

Ok... this has to be automated... lets see...

+++ grub2-gdb/kern/i386/pc/startup.S    2008-05-07 09:39:11.000000000 +0200

How about raving readmode IDT (or IDT what we started with) so we can recall it when we are calling BIOS services... just in case.

+++ grub2-gdb/term/i386/pc/serial.c     2008-05-07 09:39:11.000000000 +0200

Not really happy about this change. Lets try to figure out better interface...


Ok... Needs fine tuning for changelog entry, but otherwise fine.


I hope those keep you busy for a while :)

Vesa Jääskeläinen

reply via email to

[Prev in Thread] Current Thread [Next in Thread]