Hi Waldek,
On Wed, Nov 03, 2010, Waldemar Rymarkiewicz wrote:
> +static sim_connection_status_t sim_card_connection_status = SIM_DISCONNECTED;
Consider shortening these names. At least one of them (either the type
or the variable).
> +static void * sap_data = NULL; /* SAP server private data.*/
No whitespace between * and sap_data. Why is this void instead of a
specific type when its static and therefore wont be seen by other
modules?
> + if(sim_card_connection_status == SIM_MISSING)
Space before (
> + }else if(sim_card_connection_status != SIM_CONNECTED)
Space after } and before (
> + sap_power_sim_on_rsp(sap_device, SAP_RESULT_ERROR_NOT_ACCESSIBLE);
> + else {
> + sap_power_sim_on_rsp(sap_device, SAP_RESULT_ERROR_NO_REASON);
> + }
Why do you have {} for one branch but not for the other? In general
one-line branches/scopes shouldn't have braces, though the kernel coding
guideline does allow them if at least one part of the same if-else
statement has them.
> +static inline DBusMessage *invalid_args(DBusMessage *msg)
Never mind the previous comment I had about this. I checked the rest of
the tree and it seems this inline convention for D-Bus error generating
functions is quite common (though imho might not be needed at all).
> + if(sim_card_connection_status != SIM_CONNECTED)
Space before (
> +}
> +
> +
> +static GDBusMethodTable dummy_methods[] = {
Remove the other empty line.
> + { "OngoingCall", "b", "", ongoing_call},
> + { "MaxMessageSize", "u", "", max_msg_size},
> + { "Disconnect", "", "", disconnect},
> + { "CardStatus", "u", "", card_status},
> + { }
> +static GDBusSignalTable dummy_signals[] = {
> + { "","" },
> + { }
> +};
If you have no signals just pass NULL to g_dbus_register_interface
instead of declaring an (almost) empty table. Also, "" isn't a valid
value for the signal name.
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
