search

mate-desktop / mate-settings-daemon

MATE settings daemon
https://mate-desktop.org
GNU General Public License v2.0
44 stars 46 forks source link

Gdbus #324

Closed yetist closed 9 months ago

yetist commented 4 years ago

Please test and review.

EDIT: Please keep this PR OPEN, until this PR merged: https://github.com/mate-desktop/mate-control-center/pull/577

raveit65 commented 4 years ago

I guess m-c-c PR depends on this one. https://github.com/mate-desktop/mate-control-center/pull/577 So I just pre-bumped version of m-s-d in master. @yetist Can you please rebase PR with master?

After that m-s-d required version in m-c-c PR can be increased to 1.25.0

yetist commented 4 years ago

@raveit65

Don't wait for this PR, please release m-s-d version 1.25.0 directly.

This PR must be merged after https://github.com/mate-desktop/mate-control-center/pull/577 is merged, otherwise m-c-c compilation will be wrong.

EDIT: In this PR, m-s-d will no longer provide `/usr/include/mate-settings-daemon/ files, which will cause m-c-c 1.25.0 and below versions to fail to compile.

https://github.com/mate-desktop/mate-control-center/pull/577 will no longer directly depend on m-s-d, after m-c-c applies that PR, m-c-c can be compiled normally regardless of whether m-s-d provides mate-settings-client.h file.

raveit65 commented 4 years ago

Good, than we have less dependencies. An way, so there isn't a reason for me to release m-s-d-1.25.0 now. I will wait until this PR is merged.

raveit65 commented 4 years ago

@yetist Please give me a favor and don't use tabs when you rewrite code. Our default style is to use spaces, see wiki. Since several years we talk to external developer not to use tabs in PRs. So please be a paragon as part of mate desktop :)

sc0w commented 4 years ago

New build warnings detected:

plugins/datetime/msd-datetime-mechanism.c:745:18: warning: Either the condition 'error!=NULL' is redundant or there is possible null pointer dereference: error. [nullPointerRedundantCheck]
                *error = g_error_new (MSD_DATETIME_MECHANISM_ERROR,
                 ^

plugins/datetime/msd-datetime-mechanism.c:740:19: note: Assuming that condition 'error!=NULL' is not redundant
        if (error != NULL) {
                  ^

plugins/datetime/msd-datetime-mechanism.c:745:18: note: Null pointer dereference
                *error = g_error_new (MSD_DATETIME_MECHANISM_ERROR,
                 ^
msd-datetime-mechanism.c:197:9: warning: This statement is never executed
        tv.tv_sec += (time_t) seconds_to_add;
        ^~

msd-datetime-mechanism.c:393:13: warning: This statement is never executed
        if (g_file_test ("/sbin/hwclock",
            ^~~~~~~~~~~

msd-datetime-mechanism.c:397:92: warning: This statement is never executed
                cmd = g_strdup_printf ("/sbin/hwclock %s --systohc", using_utc ? "--utc" : "--localtime");
                                                                                           ^~~~~~~~~~~~~

msd-datetime-mechanism.c:419:83: warning: This statement is never executed
                if (!_rh_update_etc_sysconfig_clock ("UTC=", using_utc ? "true" : "false", &error)) {
                                                                                  ^~~~~~~

msd-datetime-mechanism.c:420:25: warning: This statement is never executed
                        g_dbus_method_invocation_return_gerror (invocation, error);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

msd-datetime-mechanism.c:455:30: warning: This statement is never executed
        tv.tv_sec = (time_t) arg_seconds_since_epoch;
                             ^~~~~~~~~~~~~~~~~~~~~~~

msd-datetime-mechanism.c:490:14: warning: This statement is never executed
        if (!system_timezone_set_from_file (zonefile, &error)) {
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
msd-datetime-generated.c:1493:18: warning: declaration shadows a variable in the global scope [-Wshadow]
    const gchar *timezone)
                 ^
/usr/include/time.h:175:17: note: previous declaration is here
extern long int timezone;
                ^
rbuj commented 4 years ago

@sc0w they are not build warnings, they come from the analysis tools, at least you have to say which tool has generated the report, cppcheck or clang's scan-build.

raveit65 commented 4 years ago

I will look at this when m-c-c PR is updated and merged.

sc0w commented 3 years ago 
main.c:80:32: warning: unused parameter 'sender_name' [-Wunused-parameter]
                   gchar      *sender_name,
                               ^

main.c:81:32: warning: unused parameter 'signal_name' [-Wunused-parameter]
                   gchar      *signal_name,
                               ^

main.c:82:32: warning: unused parameter 'parameters' [-Wunused-parameter]
                   GVariant   *parameters,
                               ^

main.c:83:32: warning: unused parameter 'user_data' [-Wunused-parameter]
                   gpointer    user_data)
                               ^

mate-settings-manager.c:322:56: warning: unused parameter 'user_data' [-Wunused-parameter]
_unload_plugin (MateSettingsPluginInfo *info, gpointer user_data)
                                                       ^

mate-settings-manager.c:417:43: warning: unused parameter 'name' [-Wunused-parameter]
                         const gchar     *name,
                                          ^

mate-settings-manager.c:450:40: warning: unused parameter 'connection' [-Wunused-parameter]
name_lost_handler_cb (GDBusConnection *connection,
                                       ^

mate-settings-manager.c:451:40: warning: unused parameter 'name' [-Wunused-parameter]
                      const gchar     *name,
                                       ^

mate-settings-manager.c:452:40: warning: unused parameter 'user_data' [-Wunused-parameter]
                      gpointer         user_data)
                                       ^

msd-datetime-mechanism.c:307:93: warning: unused parameter 'user_data' [-Wunused-parameter]
                                                             gpointer                       user_data)
                                                                                            ^

msd-datetime-mechanism.c:361:77: warning: unused parameter 'user_data' [-Wunused-parameter]
                                             gpointer                       user_data)
                                                                            ^

msd-datetime-mechanism.c:516:43: warning: unused parameter 'name' [-Wunused-parameter]
                         const gchar     *name,
                                          ^

msd-datetime-mechanism.c:575:40: warning: unused parameter 'connection' [-Wunused-parameter]
name_lost_handler_cb (GDBusConnection *connection,
                                       ^

msd-datetime-mechanism.c:576:40: warning: unused parameter 'name' [-Wunused-parameter]
                      const gchar     *name,
                                       ^

msd-datetime-mechanism.c:761:35: warning: unused parameter 'mechanism' [-Wunused-parameter]
  761 | _set_time (MsdDatetimeMechanism  *mechanism,
      |            ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~

msd-media-keys-manager.c: In function 'bus_acquired_handler_cb':
msd-media-keys-manager.c:1623:43: warning: unused parameter 'name' [-Wunused-parameter]
 1623 |                          const gchar     *name,
      |                          ~~~~~~~~~~~~~~~~~^~~~

msd-media-keys-manager.c: In function 'name_lost_handler_cb':
msd-media-keys-manager.c:1655:52: warning: unused parameter 'connection' [-Wunused-parameter]
 1655 | static void name_lost_handler_cb (GDBusConnection *connection,
      |                                   ~~~~~~~~~~~~~~~~~^~~~~~~~~~

msd-media-keys-manager.c:1656:52: warning: unused parameter 'name' [-Wunused-parameter]
 1656 |                                   const gchar     *name,
      |                                   ~~~~~~~~~~~~~~~~~^~~~

msd-media-keys-manager.c:1657:52: warning: unused parameter 'user_data' [-Wunused-parameter]
 1657 |                                   gpointer         user_data)
      |                                   ~~~~~~~~~~~~~~~~~^~~~~~~~~

msd-xrandr-manager.c:2704:43: warning: unused parameter 'name' [-Wunused-parameter]
 2704 |                          const gchar     *name,
      |                          ~~~~~~~~~~~~~~~~~^~~~

msd-xrandr-manager.c: In function 'name_lost_handler_cb':
msd-xrandr-manager.c:2746:40: warning: unused parameter 'connection' [-Wunused-parameter]
 2746 | name_lost_handler_cb (GDBusConnection *connection,
      |                       ~~~~~~~~~~~~~~~~~^~~~~~~~~~

msd-xrandr-manager.c:2747:40: warning: unused parameter 'name' [-Wunused-parameter]
 2747 |                       const gchar     *name,
      |                       ~~~~~~~~~~~~~~~~~^~~~

msd-xrandr-manager.c:2748:40: warning: unused parameter 'user_data' [-Wunused-parameter]
 2748 |                       gpointer         user_data)
      |                       ~~~~~~~~~~~~~~~~~^~~~~~~~~
sc0w commented 3 years ago

@yetist Thanks, I noticed these warnings too:

main.c:79:32: warning: unused parameter 'proxy' [-Wunused-parameter]
   79 | on_session_signal (GDBusProxy *proxy,
      |                    ~~~~~~~~~~~~^~~~~

main.c:90:32: warning: unused parameter 'sender_name' [-Wunused-parameter]
   90 |                    gchar      *sender_name,
      |                    ~~~~~~~~~~~~^~~~~~~~~~~

main.c:92:32: warning: unused parameter 'parameters' [-Wunused-parameter]
   92 |                    GVariant   *parameters,
      |                    ~~~~~~~~~~~~^~~~~~~~~~

mate-settings-manager.c: In function 'mate_settings_manager_start_handler':
mate-settings-manager.c:371:61: warning: unused parameter 'user_data' [-Wunused-parameter]
  371 |                                      gpointer               user_data)
      |                                      ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~
raveit65 commented 3 years ago

Did someone really test this PR ?

lukefromdc commented 3 years ago

It didn't occur to me to test media player controls with this when I restarted m-s-d with it, in fact those particular controls I rarely use. This is why more testers with different workflows is so important

raveit65 commented 3 years ago

Not only media-keys are broken. /usr/libexec/mate-settings-daemon --replace command is broken too!

raveit65 commented 3 years ago

@yetist This PR has a few issue. See my previous comment.

raveit65 commented 3 years ago

@yetist PR has a file conflict with latest commit in master. And media-keys doesn't work, see my comments.

yetist commented 3 years ago

Now it can run, I need to fix some build warnings later.

raveit65 commented 3 years ago

@yetist Any chance to start work again on this PR and fixing the media-keys issue and runtime warnings? See my origin post.

yetist commented 1 year ago

This patch series is so big that I will split them into different PRs.

raveit65 commented 10 months ago

@yetist Any chance to spilt out more into different PRs. Port to GDbus seems to be important.

yetist commented 10 months ago

@yetist Any chance to spilt out more into different PRs. Port to GDbus seems to be important.

ok, I will do.

yetist commented 9 months ago

@mate-desktop/core-team

Please review this PR, thanks.