DirectFB UI: auto create a default ~/.directfbrc if missing

[g-reno]

DirectFB UI will crash if you don't have a ~/.directfbrc file in place.

FreeRDP should automatically create a default ~/.directfbrc file if one is not found.

[awakecoding]

It will also crash if the resolution used by FreeRDP isn't the same as the one in the directfbrc file if there is one

are you sure that generating a default file is the way to go? DirectFB is not meant for everyday users

[g-reno]

That's true but when someone does use it it shouldn't crash on them first thing.

If there is no default resource file the app should output one and log it on the console.

And the app should also check the resolutions and just error out with a message if they are not compatible.

[nils-a]

2011/6/3, g-reno reply@reply.github.com:

DirectFB UI will crash if you don't have a ~/.directfbrc file in place.

FreeRDP should automatically create a default ~/.directfbrc file if one is not found.

Reply to this email directly or view it on GitHub: https://github.com/FreeRDP/FreeRDP/issues/32

Gesendet von meinem Mobilgerät

[otavio]

The dfbfreerdp could update the file depending on the comand line args.

[g-reno]

Here is a patch that adds auto creating a default ~/.directfbrc resource file:

[otavio]

This patch seems a good start but we need to handle the system backend someway. You're using x11 but people need to be able to use other and I can think of the need of not overwrite the .directfbrc file et all (specially for debugging and like).

[g-reno]

I can change system=X11 to system=fbdev (still Linux)

Not sure about allowable values but I guess that would be fbdev, opengl, directx, sdl. Anything special for mac? How do we query for the graphic system that is being used? I guess we can just pick one based on the OS, after all this is just for creating a 'default' resource file.

And as long as a ~/.directfbrc file exists the code will not overwrite it. See the code comment.

[otavio]

I'd say to overwrite it except in case you use an option to forbid it. I also think fbdev ought to be used by default and you could have an option to overwrite it to use other (in case x11 for testing).

[g-reno]

No overwriting. That makes things very complicated.

All that is necessary is to create a 'default' resource file for the particular OS if one does not already exist.

If a resource file already exists we do nothing.

So for testing you just create whatever ~/.directfbrc you want for your testing and then it will not get overwritten.

[otavio]

In theory, this works. But I think this is not user friendly.

[g-reno]

Well, it's a whole lot more user friendly than having the system crash on them which is exactly what happens today.
And I mean it can lock up their workstation currently.

It is not necessary to have a bunch of controlling command line options or anything like that.
You create resource files using an editor not the app.

This is just to make sure that there is at least a default ~/.directfbrc in place so the user does not experience a crash.

[otavio]

I agree that it is more user friendly but FreeRDP can handle with it more properly IMO and even tought you don't use the command line options it seems more user friendly to me and easier to understand by new users.

I won't merge this patch this way. If someone else wants to do that, feels free.

[g-reno]

Updated patch for setting default graphics system in the default ~/.directfbrc resource file:

[awakecoding]

@otavio: I disagree with the idea of overwriting the file, or adding command-line options to tune how this default file is generated

I think the only acceptable action regarding the directfbrc file is to generate a default one if no such file exists. The rest can be modified with a text editor.

@g-reno:

AFAIC, DirectFB is not available on Windows, and it has been broken on Mac OS X for a while. I think we can dumb it down to just Linux, so no need to detect Windows or Mac OS X. In any case, the DirectFB port is not really something meant to be used by end users, mostly by people developing systems to be used by others, where those users usually don't even know they're using software that runs on top of DirectFB.

My main concern is when DirectFB is used with a real framebuffer device, and when it is used with the X11 backend. Your code seem to already check for that, so keep that part, but ditch the OS detection part which I guess you haven't tested anyway because I doubt it would work even if we get to compile dfbfreerdp on those platforms.

For comments, use /* */ instead of // as instructed in the coding guidelines: http://www.freerdp.com/wiki/doku.php?id=coding_guidelines

Also, why do something like this:

static char resourcefile[512]; strncat(resourcefile, home, strlen(home)); resourcefile[512-1] = (char)0;

I see that you want to make the last character of that array a null character, but that doesn't do much. strncat adds a null-terminating character according to this page: http://www.cplusplus.com/reference/clibrary/cstring/strncat/

Plus, if strncat wasn't adding a null terminating character, then you'd still have a problem because you would like you null character to be at the end of the string, not at the end of the full array.

[otavio]

@awakecoding in this case I think the software ought to warn the user that it is using the file and that he/she needs to change it if need.

[awakecoding]

@otavio:

what if it generates a file with default values, warning the user about which values it is using, and that they might manual adjustements later on?

[g-reno]

OS detection: Windows was added because I found references of people attempts to port DirectFB to Windows. I can remove Windows. But, there is also X11 available on Mac. I don't have a Mac right now but I think some detection should be kept here. I'll make some changes and then maybe someone with Mac could test them.

Commenting: I'll change comment style. Is there some particular reason that C++ style '//' comments are not allowable? You find them almost everywhere now.

strncat will bite you hard if you happen to cat in more than the buffer size. Then you will not have a terminating null. That is why I force a terminating null just in case. It's just there in case you've filled the whole buffer.

The server puts out this message that alerts the user that a default resource file was created:

$ dfb/dfbfreerdp -a 16 --gdi sw 192.168.2.49
INFO: created default resource file: /home/greno/.directfbrc
starting thread 1 to 192.168.2.49:3389
run_dfbfreerdp:
DBG main (767): main thread, waiting for all threads to exit
keyboard_layout: 0
connecting to 192.168.2.49:3389
connecting to 192.168.2.49:3389
connecting to 192.168.2.49:3389
Standard RDP encryption negotiated

DirectFB does not output any information about what resource file(s) it is using. Fairly typical.

.

[awakecoding]

@g-reno:

comments: you said it yourself, those are C++ style comments, while this is pure C. Plus, these are in the coding guidelines, which have been discussed already.

As for Mac OS X, I do have a Mac, and if it were to compile the backend would most likely be "x11", not opengl, to run on the Mac OS X X11 server (X11.app).

The DirectFB port doesn't work on Mac simply because DirectFB doesn't work on Mac, at least not recently. I tried getting it to work, but apparently it used to work on Mac OS X and nobody maintained it for a while.

[otavio]

For me it works. I am experienced enough to look into code and understand what's going on.

I am unsure if this is how it ought to work for general users. I'd say, no; but ... it doesn't matter for me et all.

[g-reno]

@awakecoding: Yes while working on this patch I saw your post about Mac OS X not compiling DirectFB. It looks like you ran into the recursive mutex pthreads macro problem that bit BSD and OS X. That was fixed a couple years ago I thought but looks like maybe there was a regression in pthreads. That affects more than just Mac OS X w/DirectFB so I would think it will get fixed again.

.

[g-reno]

Third patch update for setting default graphics system in the default ~/.directfbrc resource file:

[g-reno]

https://github.com/FreeRDP/FreeRDP/pull/40 Patch merged in this pull-request issue.