Hi Julien,
comments on the code are below.
On 05/22/2016 07:46 PM, Julien Gueytat wrote:
It works exactly the same way than the angle parameter:
* --angle <-> /etc/rotation file
* --fbdev <-> /etc/fbdev file
Signed-off-by: Julien Gueytat <cont...@jgueytat.fr>
---
psplash-fb.c | 16 ++++++++++------
psplash-fb.h | 4 ++--
psplash.c | 57 ++++++++++++++++++++++++++++++++-------------------------
3 files changed, 44 insertions(+), 33 deletions(-)
diff --git a/psplash-fb.c b/psplash-fb.c
index 8daaf6f..d344e5a 100644
--- a/psplash-fb.c
+++ b/psplash-fb.c
@@ -99,18 +99,20 @@ attempt_to_change_pixel_format (PSplashFB *fb,
}
PSplashFB*
-psplash_fb_new (int angle)
+psplash_fb_new (int angle, int fbdev_id)
{
struct fb_var_screeninfo fb_var;
struct fb_fix_screeninfo fb_fix;
int off;
- char *fbdev;
+ char fbdev[9] = "/dev/fb0";
PSplashFB *fb = NULL;
- fbdev = getenv("FBDEV");
- if (fbdev == NULL)
- fbdev = "/dev/fb0";
+ if (fbdev_id > 0 && fbdev_id < 10)
+ {
+ // Conversion from integer to ascii.
+ fbdev[7] = fbdev_id + 48;
+ }
You removed the possiblity to get the fb device from FBDEV!
Please don't to that as people may rely on that feature!
If you like to add a fbdev commandline option make sure it can co-exist
with the already available methods for setting this option.
And don't forget to add documentation!
if ((fb = malloc (sizeof(PSplashFB))) == NULL)
{
...
@@ -205,35 +205,41 @@ int
main (int argc, char** argv)
{
char *tmpdir;
- int pipe_fd, i = 0, angle = 0, ret = 0;
+ int pipe_fd, i = 0, angle = 0, fbdev_id = 0, ret = 0;
PSplashFB *fb;
bool disable_console_switch = FALSE;
-
+
signal(SIGHUP, psplash_exit);
signal(SIGINT, psplash_exit);
signal(SIGQUIT, psplash_exit);
- while (++i < argc)
- {
- if (!strcmp(argv[i],"-n") || !strcmp(argv[i],"--no-console-switch"))
- {
- disable_console_switch = TRUE;
- continue;
- }
+ while (++i < argc) {
+ if (!strcmp(argv[i],"-n") || !strcmp(argv[i],"--no-console-switch"))
+ {
+ disable_console_switch = TRUE;
+ continue;
+ }
+
+ if (!strcmp(argv[i],"-a") || !strcmp(argv[i],"--angle"))
+ {
+ if (++i >= argc) goto fail;
+ angle = atoi(argv[i]);
+ continue;
+ }
+
+ if (!strcmp(argv[i],"-f") || !strcmp(argv[i],"--fbdev"))
+ {
+ if (++i >= argc) goto fail;
+ fbdev_id = atoi(argv[i]);
+ continue;
+ }
- if (!strcmp(argv[i],"-a") || !strcmp(argv[i],"--angle"))
- {
- if (++i >= argc) goto fail;
- angle = atoi(argv[i]);
- continue;
- }
-
fail:
fprintf(stderr,
- "Usage: %s [-n|--no-console-switch][-a|--angle <0|90|180|270>]\n",
- argv[0]);
+ "Usage: %s [-n|--no-console-switch][-a|--angle <0|90|180|270>][-f|--fbdev
<0..9>]\n",
+ argv[0]);
exit(-1);
- }
+ }
Please use the same coding style everywhere in psplash.
Furthermore please avoid/remove trailing whitespaces in new/changed code
tmpdir = getenv("TMPDIR");
@@ -245,10 +251,10 @@ main (int argc, char** argv)
if (mkfifo(PSPLASH_FIFO, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP))
{
if (errno!=EEXIST)
- {
- perror("mkfifo");
- exit(-1);
- }
+ {
+ perror("mkfifo");
+ exit(-1);
+ }
}
This change is irrelevant for new fbdev option.
Please send this whitespace fix as a separate patch.
pipe_fd = open (PSPLASH_FIFO,O_RDONLY|O_NONBLOCK);
@@ -262,10 +268,11 @@ main (int argc, char** argv)
if (!disable_console_switch)
psplash_console_switch ();
- if ((fb = psplash_fb_new(angle)) == NULL) {
+ if ((fb = psplash_fb_new(angle,fbdev_id)) == NULL)
+ {
ret = -1;
goto fb_fail;
- }
+ }
/* Clear the background with #ecece1 */
psplash_fb_draw_rect (fb, 0, 0, fb->width, fb->height,
IMHO in this case fixing the whitespace is fine because the if clause
needs to be adopted for the new option.
