Re: [PATCH] kernel/syscall: neue Funktion für das Registrieren von Kernel-Syscalls

[Kevin Wolf <kevin@xxxxxxxxxx>]

Am 18.02.2021 um 21:03 hat MNemo GMX geschrieben:
> Hi Nico,
> 
> On 2/18/21 10:42 AM, Nico Mayer wrote:
> > * Aktuell gibt es lediglich einen Kernel-Syscall(KERN_SYSCALL_PM_SLEEP).
> > Dieser Syscall wurde bisher manuell in die globale Syscall-Liste
> > eingetragen. Um das Registrieren von Kernel-Syscalls zu vereinfachen,
> > implementiert dieser Commit die Funktion syscall_register_full().
> > ---
> >   src/kernel/include/syscall.h |  3 +++
> >   src/kernel/src/syscall.c     | 23 ++++++++++++++++++-----
> >   2 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/kernel/include/syscall.h b/src/kernel/include/syscall.h
> > index ad941e85..cd74ac4f 100644
> > --- a/src/kernel/include/syscall.h
> > +++ b/src/kernel/include/syscall.h
> > @@ -70,6 +70,9 @@ void syscall_arch(machine_state_t* isf);
> >   /// Einen neuen Syscall verfuegbar machen
> >   void syscall_register(syscall_arg_t number, void* handler, size_t arg_count);
> > 
> > +/// Einen neuen Syscall verfuegbar machen
> > +void syscall_register_full(syscall_arg_t number, void* handler, size_t arg_count, bool privileged);
> > +
> >   /// Syscalls initialisieren
> >   void syscall_init(void);
> > 
> > diff --git a/src/kernel/src/syscall.c b/src/kernel/src/syscall.c
> > index f74dbebe..2f0274b0 100644
> > --- a/src/kernel/src/syscall.c
> > +++ b/src/kernel/src/syscall.c
> > @@ -151,11 +151,8 @@ void syscall_init()
> >       syscall_register(SYSCALL_LIO_SRV_OP_DONE, &syscall_lio_srv_op_done, 3);
> >       syscall_register(SYSCALL_LIO_SRV_WAIT, &syscall_lio_srv_wait, 0);
> > 
> > -    syscalls[KERN_SYSCALL_PM_SLEEP] = (syscall_t) {
> > -        .handler    = &kern_syscall_pm_sleep,
> > -        .arg_count  = 2,
> > -        .privileged = true,
> > -    };
> > +
> > +	syscall_register_full(KERN_SYSCALL_PM_SLEEP, &kern_syscall_pm_sleep, 2, true);
> 
> Hier ist mit einem TAB eingerückt. Bitte einheitlich mit 'SPACES' einrücken.

Ah, gut, dass noch einer aufpasst. Das hätte ich womöglich übersehen.
Ja, Einrückung mit vier Spaces bitte in tyndur.

Die zusätzliche Leerzeile brauchen wir hier auch nicht, das wären zwei
hintereinander.

> >   }
> > 
> >   /**
> > @@ -178,3 +175,19 @@ void syscall_register(syscall_arg_t number, void* handler, size_t arg_count)
> >       syscalls[number].arg_count = arg_count;
> >       syscalls[number].privileged = false;
> >   }
> > +
> > +/**
> > + * Einen neuen Syscall registrieren
> > + *

Hier hast du ein Leerzeichen am Zeilenende, das da nicht hingehört (git
show oder git diff machen dir das auch schön rot, das schaue ich selber
regelmäßig nochmal an, bevor ich einen Patch abschicke - aber hilfreich
ist es, wenn auch der Editor das schon farblich markiert).

> > + * @param number Syscallnummer (syscallno.h!)
> > + * @param handler Adresse der Handler-Funktion
> > + * @param arg_count Anzahl der Argumente, die fuer diesen Syscall uebergeben
> > + *                  werden muessen.
> > + * @param privileged Bei 'true' darf der Syscall nur durch einen

Wieder eine Zeile mit Trailing Whitespace.

> > + *					 Kernel-Thread verwendet werden.

Und Tabs bei der Einrückung.

> > + */
> > +void syscall_register_full(syscall_arg_t number, void* handler, size_t arg_count, bool privileged)
> > +{
> > +	syscall_register(number, handler, arg_count);
> > +    syscalls[number].privileged = privileged;
> 
> Hier auch das Einrücken.
> 
> Würde das auch umdrehen. Das die spezielle Funktion die allgemeine aufruft
> 
> syscall_register(...) -> syscall_register_full(..., false);

So hatte ich das im IRC vorgeschlagen, ja.

Kevin