I recently finished an implementation of vprintf for my kernel. It does NOT do floating-point or octal conversions, but will handle everything else. Here is a link to my code:
vprintf.c
Please comment with thoughts, criticisms, or recommendations for my work.
vprintf function implementation
-
MakaAlbarn001
- Posts: 20
- Joined: Fri Jun 16, 2017 1:51 am
- Libera.chat IRC: Maka_Albarn
-
Schol-R-LEA
- Member
- Posts: 1925
- Joined: Fri Oct 27, 2006 9:42 am
- Location: Athens, GA, USA
Re: vprintf function implementation
My first thought, which is a recommendation that is commonly given for this sort of project, is to give the kernel versions of such functions a unique name, different from their standard, userspace counterparts. A typical example in this instance might be kvprintf(). This makes it clear to anyone else reading the code that they are specifically for kernel use.
Another general comment would be that you might want to separate the formatting into a kvsprintf() function, so that it could be re-used for any other string-handling functions. The kvprintf() would then become a call to ksprintf() and then a call to kputs(), or something similar to that anyway. De-coupling the output from the string-handling would also improve the efficiency of both operations, and thus the operation overall (even given the function call overhead that this would add).
As an aside, I am puzzled as to why you are posting these as Gists, rather than creating a fully-fledged Github repo for your project. I realize you may have a reason for this, but it is a bit unusual.
Anyway, I don't have time to look at the code in detail yet, but I will try to get back to it when I get the chance.
Another general comment would be that you might want to separate the formatting into a kvsprintf() function, so that it could be re-used for any other string-handling functions. The kvprintf() would then become a call to ksprintf() and then a call to kputs(), or something similar to that anyway. De-coupling the output from the string-handling would also improve the efficiency of both operations, and thus the operation overall (even given the function call overhead that this would add).
As an aside, I am puzzled as to why you are posting these as Gists, rather than creating a fully-fledged Github repo for your project. I realize you may have a reason for this, but it is a bit unusual.
Anyway, I don't have time to look at the code in detail yet, but I will try to get back to it when I get the chance.
Rev. First Speaker Schol-R-LEA;2 LCF ELF JAM POEE KoR KCO PPWMTF
Ordo OS Project
Lisp programmers tend to seem very odd to outsiders, just like anyone else who has had a religious experience they can't quite explain to others.
Ordo OS Project
Lisp programmers tend to seem very odd to outsiders, just like anyone else who has had a religious experience they can't quite explain to others.
Re: vprintf function implementation
Well, floating-point may be overkill for the kernel, anyway, since the kernel is not supposed to access the FPU except during context switch. Plus, printing FP numbers requires a whole lot of stack if you want to do it right.
There is altogether too much code in your implementation. Too much duplicated code. You can print any signed integer type the same way, you only need to adjust the type given to va_arg(). intmax_t is definitely big enough to hold all integers you might be presented with, so you need that code only once. You have the code to print strings available four times, with minor variations. But the only difference between having a precision and not having one before a %s is an upper limit to the string length you will access. So the length of the string is either given by strlen() or strnlen(...,precision). And that is the whole difference.
Also, your code allows too much. You allow flags, widths, precisions, and length specifiers in any order and any amount. That might also be a reason for the code size.
I would suggest taking a look at musl's implementation. That's a nice and small one. And easy to adopt for kernel use. And permissively licensed.
There is altogether too much code in your implementation. Too much duplicated code. You can print any signed integer type the same way, you only need to adjust the type given to va_arg(). intmax_t is definitely big enough to hold all integers you might be presented with, so you need that code only once. You have the code to print strings available four times, with minor variations. But the only difference between having a precision and not having one before a %s is an upper limit to the string length you will access. So the length of the string is either given by strlen() or strnlen(...,precision). And that is the whole difference.
Also, your code allows too much. You allow flags, widths, precisions, and length specifiers in any order and any amount. That might also be a reason for the code size.
I would suggest taking a look at musl's implementation. That's a nice and small one. And easy to adopt for kernel use. And permissively licensed.
Carpe diem!