[PATCH skeleton 4/5] Fix compiler warnings for objects/*

Brad Bishop bradleyb at fuzziesquirrel.com
Fri Apr 22 00:15:06 AEST 2016


Thanks Cyril!


> On Apr 17, 2016, at 8:37 PM, Cyril Bur <cyrilbur at gmail.com> wrote
> e:
> 
> On Thu, 14 Apr 2016 11:00:54 -0500
> OpenBMC Patches <openbmc-patches at stwcx.xyz> wrote:
> 
>> From: Brad Bishop <bradleyb at us.ibm.com>
>> 
>> ---
>> includes/openbmc.h              |  2 +-
>> objects/board_vpd_obj.c         |  3 +-
>> objects/button_power_obj.c      |  4 +--
>> objects/button_reset_obj.c      |  4 +--
>> objects/control_host_obj.c      |  1 -
>> objects/flash_bios_obj.c        | 65 +++++------------------------------------
>> objects/flasher_obj.c           | 46 +----------------------------
>> objects/host_watchdog_obj.c     |  3 +-
>> objects/hwmons_barreleye.c      |  5 ++--
>> objects/led_controller.c        | 17 +++++++----
>> objects/pcie_slot_present_obj.c |  6 +---
>> objects/power_control_obj.c     |  3 --
>> 12 files changed, 28 insertions(+), 131 deletions(-)
>> 
>> diff --git a/includes/openbmc.h b/includes/openbmc.h
>> index a6d420b..e338dda 100644
>> --- a/includes/openbmc.h
>> +++ b/includes/openbmc.h
>> @@ -20,7 +20,7 @@
>> #ifdef __arm__
>> static inline void devmem(void* addr, uint32_t val)
>> {
>> -	printf("devmem 0x%08x = 0x%08x\n",addr,val);
>> +	printf("devmem 0x%08x = 0x%08x\n",(uint32_t)addr,val);
> 
> You know printf can print pointers directly with %p? Seems nicer than a
> cast…

agreed.

> 
>>         asm volatile("" : : : "memory");
>>         *(volatile uint32_t *)addr = val;
>> }
>> diff --git a/objects/board_vpd_obj.c b/objects/board_vpd_obj.c
>> index a6576db..c64c250 100644
>> --- a/objects/board_vpd_obj.c
>> +++ b/objects/board_vpd_obj.c
>> @@ -12,7 +12,6 @@ main(gint argc, gchar *argv[])
>> 	GDBusProxy *p;
>> 	GError *error;
>> 	GVariant *parm;
>> -	GVariant *result;
>> 
>> 	loop = g_main_loop_new(NULL, FALSE);
>> 
>> @@ -44,7 +43,7 @@ main(gint argc, gchar *argv[])
>> 	parm = g_variant_new("(v)",dict);
>> 
>> 	error = NULL;
>> -	result = g_dbus_proxy_call_sync(p,
>> +	(void)g_dbus_proxy_call_sync(p,
> 
> Are we compiling with the option to warn about ignoring return values? I'm not
> strictly opposed to explicitly stating in the code that we ignore return values
> like this but I'm a bigger fan of consistency. Is is something we want to push
> for?

-Wall doesn’t seem to mind if the casts aren’t there.  Removed.

> 
>> 			"update",
>> 			parm,
>> 			G_DBUS_CALL_FLAGS_NONE,
>> diff --git a/objects/button_power_obj.c b/objects/button_power_obj.c
>> index f9a2b09..d1dbef0 100644
>> --- a/objects/button_power_obj.c
>> +++ b/objects/button_power_obj.c
>> @@ -44,7 +44,7 @@ on_button_interrupt( GIOChannel *channel,
>> 	gchar buf[2];
>> 	buf[1] = '\0';
>> 	g_io_channel_seek_position( channel, 0, G_SEEK_SET, 0 );
>> -	GIOStatus rc = g_io_channel_read_chars( channel,
>> +	(void)g_io_channel_read_chars(channel,
>> 			buf, 1,
>> 			&bytes_read,
>> 			&error );
>> @@ -84,9 +84,7 @@ on_bus_acquired(GDBusConnection *connection,
>> {
>> 	ObjectSkeleton *object;
>> 	//g_print ("Acquired a message bus connection: %s\n",name);
>> -	cmdline *cmd = user_data;
>> 	manager = g_dbus_object_manager_server_new(dbus_object_path);
>> -	int i=0;
>> 	gchar *s;
>> 	s = g_strdup_printf("%s/%s",dbus_object_path,instance_name);
>> 	object = object_skeleton_new(s);
>> diff --git a/objects/button_reset_obj.c b/objects/button_reset_obj.c
>> index 6e915a5..22e98c5 100644
>> --- a/objects/button_reset_obj.c
>> +++ b/objects/button_reset_obj.c
>> @@ -44,7 +44,7 @@ on_button_interrupt( GIOChannel *channel,
>> 	gchar buf[2];
>> 	buf[1] = '\0';
>> 	g_io_channel_seek_position( channel, 0, G_SEEK_SET, 0 );
>> -	GIOStatus rc = g_io_channel_read_chars( channel,
>> +	(void)g_io_channel_read_chars( channel,
>> 			buf, 1,
>> 			&bytes_read,
>> 			&error );
>> @@ -84,9 +84,7 @@ on_bus_acquired(GDBusConnection *connection,
>> {
>> 	ObjectSkeleton *object;
>> 	//g_print ("Acquired a message bus connection: %s\n",name);
>> -	cmdline *cmd = user_data;
>> 	manager = g_dbus_object_manager_server_new(dbus_object_path);
>> -	int i=0;
>> 	gchar *s;
>> 	s = g_strdup_printf("%s/%s",dbus_object_path,instance_name);
>> 	object = object_skeleton_new(s);
>> diff --git a/objects/control_host_obj.c b/objects/control_host_obj.c
>> index 3e7f2ae..e8d1745 100644
>> --- a/objects/control_host_obj.c
>> +++ b/objects/control_host_obj.c
>> @@ -182,7 +182,6 @@ on_bus_acquired(GDBusConnection *connection,
>> {
>> 	ObjectSkeleton *object;
>> 	//g_print ("Acquired a message bus connection: %s\n",name);
>> -	cmdline *cmd = user_data;
>> 	manager = g_dbus_object_manager_server_new(dbus_object_path);
>> 
>> 	gchar *s;
>> diff --git a/objects/flash_bios_obj.c b/objects/flash_bios_obj.c
>> index e661545..a46c5cb 100644
>> --- a/objects/flash_bios_obj.c
>> +++ b/objects/flash_bios_obj.c
>> @@ -1,6 +1,8 @@
>> #include <stdio.h>
>> #include <stdbool.h>
>> #include <string.h>
>> +#include <sys/wait.h>
>> +#include <sys/types.h>
>> #include "interfaces/openbmc_intf.h"
>> #include "openbmc.h"
>> #include "object_mapper.h"
>> @@ -9,8 +11,6 @@
>> static const gchar* dbus_object_path = "/org/openbmc/control/flash";
>> static const gchar* dbus_name = "org.openbmc.control.Flash";
>> static const gchar* FLASHER_BIN = "flasher.exe";
>> -static const gchar* DLOAD_BUS = "org.openbmc.managers.Download";
>> -static const gchar* DLOAD_OBJ = "/org/openbmc/managers/Download";
>> 
>> static GDBusObjectManagerServer *manager = NULL;
>> 
>> @@ -142,9 +142,8 @@ on_error(Flash *flash,
>> 		gchar* error_msg,
>> 		gpointer user_data)
>> {
>> -	int rc = 0;
>> 	SharedResource *lock = object_get_shared_resource((Object*)user_data);
>> -	gboolean locked = shared_resource_get_lock(lock);
>> +	(void)shared_resource_get_lock(lock);
>> 	flash_set_status(flash, error_msg);
>> 	flash_complete_error(flash,invocation);
>> 	printf("ERROR: %s.  Clearing locks\n",error_msg);
>> @@ -161,7 +160,7 @@ on_done(Flash *flash,
>> {
>> 	int rc = 0;
>> 	SharedResource *lock = object_get_shared_resource((Object*)user_data);
>> -	gboolean locked = shared_resource_get_lock(lock);
>> +	(void)shared_resource_get_lock(lock);
>> 	flash_set_status(flash, "Flash Done");
>> 	flash_complete_done(flash,invocation);
>> 	printf("Flash Done. Clearing locks\n");
>> @@ -221,9 +220,9 @@ on_flash_progress(GDBusConnection* connection,
>> 		gpointer user_data)
>> {
>> 	Flash *flash = object_get_flash((Object*)user_data);
>> -	SharedResource *lock = object_get_shared_resource((Object*)user_data);
>> +	(void)object_get_shared_resource((Object*)user_data);
>> 	GVariantIter *iter = g_variant_iter_new(parameters);
>> -	GVariant* v_filename = g_variant_iter_next_value(iter);
>> +	(void)g_variant_iter_next_value(iter);
>> 	GVariant* v_progress = g_variant_iter_next_value(iter);
>> 
>> 	uint8_t progress = g_variant_get_byte(v_progress);
>> @@ -235,55 +234,6 @@ on_flash_progress(GDBusConnection* connection,
>> }
>> 
>> static void
>> -on_flash_done(GDBusConnection* connection,
>> -		const gchar* sender_name,
>> -		const gchar* object_path,
>> -		const gchar* interface_name,
>> -		const gchar* signal_name,
>> -		GVariant* parameters,
>> -		gpointer user_data)
>> -{
>> -	Flash *flash = object_get_flash((Object*)user_data);
>> -	SharedResource *lock = object_get_shared_resource((Object*)user_data);
>> -	printf("Flash succeeded; unlocking flash\n");
>> -	shared_resource_set_lock(lock,false);
>> -	shared_resource_set_name(lock,"");
>> -	flash_set_status(flash,"Flash Done");
>> -}
>> -
>> -static void
>> -on_flash_error(GDBusConnection* connection,
>> -		const gchar* sender_name,
>> -		const gchar* object_path,
>> -		const gchar* interface_name,
>> -		const gchar* signal_name,
>> -		GVariant* parameters,
>> -		gpointer user_data)
>> -{
>> -	Flash *flash = object_get_flash((Object*)user_data);
>> -	SharedResource *lock = object_get_shared_resource((Object*)user_data);
>> -	printf("Flash Error; unlocking flash\n");
>> -	shared_resource_set_lock(lock,false);
>> -	shared_resource_set_name(lock,"");
>> -}
>> -
>> -static void
>> -on_download_error(GDBusConnection* connection,
>> -		const gchar* sender_name,
>> -		const gchar* object_path,
>> -		const gchar* interface_name,
>> -		const gchar* signal_name,
>> -		GVariant* parameters,
>> -		gpointer user_data)
>> -{
>> -	Flash *flash = object_get_flash((Object*)user_data);
>> -	SharedResource *lock = object_get_shared_resource((Object*)user_data);
>> -	printf("ERROR: FlashBios:  Download error; clearing flash lock\n");
>> -	shared_resource_set_lock(lock,false);
>> -	shared_resource_set_name(lock,"");
>> -}
>> -
>> -static void
>> on_bus_acquired(GDBusConnection *connection,
>> 		const gchar *name,
>> 		gpointer user_data)
>> @@ -296,8 +246,7 @@ on_bus_acquired(GDBusConnection *connection,
>> 	//TODO: don't use fixed buffer
>> 	char flasher_path[512];
>> 	memset(flasher_path, '\0', sizeof(flasher_path));
>> -	bool found = false;
>> -	gchar *flasher_file;
>> +	gchar *flasher_file = NULL;
>> 	int c = strlen(cmd->argv[0]);
>> 	while(c>0)  
>> 	{
>> diff --git a/objects/flasher_obj.c b/objects/flasher_obj.c
>> index 2d80847..1f1e5a5 100644
>> --- a/objects/flasher_obj.c
>> +++ b/objects/flasher_obj.c
>> @@ -24,7 +24,6 @@
>> #include "includes/openbmc.h"
>> 
>> static const gchar* dbus_object_path = "/org/openbmc/control";
>> -static const gchar* dbus_object_name = "Flasher_0";
>> static const gchar* dbus_name = "org.openbmc.control.Flasher";
>> 
>> static GDBusObjectManagerServer *manager = NULL;
>> @@ -33,10 +32,7 @@ static GDBusObjectManagerServer *manager = NULL;
>> 
>> #define PFLASH_VERSION	"0.8.6"
>> 
>> -static bool must_confirm = false;
>> -static bool dummy_run;
>> static bool need_relock;
>> -static bool bmc_flash;
>> #ifdef __powerpc__
>> static bool using_sfc;
>> #endif
>> @@ -54,7 +50,6 @@ static int32_t			ffs_index = -1;
>> static uint8_t FLASH_OK = 0;
>> static uint8_t FLASH_ERROR = 0x01;
>> static uint8_t FLASH_SETUP_ERROR = 0x02;
>> -static struct blocklevel_device *bl;
>> 
>> static int
>> erase_chip(void)
>> @@ -80,7 +75,6 @@ flash_message(GDBusConnection* connection,char* obj_path,char* method, char* err
>> 	GDBusProxy *proxy;
>> 	GError *error;
>> 	GVariant *parm = NULL;
>> -	GVariant *result;
>> 	error = NULL;
>> 	proxy = g_dbus_proxy_new_sync(connection,
>> 			G_DBUS_PROXY_FLAGS_NONE,
>> @@ -96,7 +90,7 @@ flash_message(GDBusConnection* connection,char* obj_path,char* method, char* err
>> 	if(strcmp(method,"error")==0) {
>> 		parm = g_variant_new("(s)",error_msg);
>> 	}
>> -	result = g_dbus_proxy_call_sync(proxy,
>> +	(void)g_dbus_proxy_call_sync(proxy,
>> 			method,
>> 			parm,
>> 			G_DBUS_CALL_FLAGS_NONE,
>> @@ -167,44 +161,6 @@ program_file(FlashControl* flash_control, const char *file, uint32_t start, uint
>> }
>> 
>> static void
>> -do_read_file(const char *file, uint32_t start, uint32_t size)
>> -{
>> -	int fd, rc;
>> -	ssize_t len;
>> -	uint32_t done = 0;
>> -
>> -	fd = open(file, O_WRONLY | O_TRUNC | O_CREAT, 00666);
>> -	if(fd == -1) {
>> -		perror("Failed to open file");
>> -		exit(1);
>> -	}
>> -	printf("Reading to \"%s\" from 0x%08x..0x%08x !\n",
>> -			file, start, size);
>> -
>> -	progress_init(size >> 8);
>> -	while(size) {
>> -		len = size > FILE_BUF_SIZE ? FILE_BUF_SIZE : size;
>> -		rc = flash_read(fl_chip, start, file_buf, len);
>> -		if(rc) {
>> -			fprintf(stderr, "Flash read error %d for"
>> -					" chunk at 0x%08x\n", rc, start);
>> -			exit(1);
>> -		}
>> -		rc = write(fd, file_buf, len);
>> -		if(rc < 0) {
>> -			perror("Error writing file");
>> -			exit(1);
>> -		}
>> -		start += len;
>> -		size -= len;
>> -		done += len;
>> -		progress_tick(done >> 8);
>> -	}
>> -	progress_end();
>> -	close(fd);
>> -}
>> -
>> -static void
>> flash_access_cleanup_bmc(void)
>> {
>> 	if(ffsh)
>> diff --git a/objects/host_watchdog_obj.c b/objects/host_watchdog_obj.c
>> index 0b3b149..2e0ef15 100644
>> --- a/objects/host_watchdog_obj.c
>> +++ b/objects/host_watchdog_obj.c
>> @@ -40,6 +40,7 @@ remove_watchdog(void)
>> 		g_source_remove(watchdogid);
>> 		watchdogid = 0;
>> 	}
>> +	return TRUE;
>> }
>> 
>> static gboolean
>> @@ -51,6 +52,7 @@ set_poll_interval(Watchdog *wd,
>> 	g_print("Setting watchdog poll interval to: %d\n", interval);
>> 	watchdog_set_poll_interval(wd, interval);
>> 	watchdog_complete_set(wd,invocation);
>> +	return TRUE;
>> }
>> 
>> static gboolean
>> @@ -93,7 +95,6 @@ on_bus_acquired(GDBusConnection *connection,
>> 		const gchar *name,
>> 		gpointer user_data)
>> {
>> -	cmdline *cmd = user_data;
>> 	manager = g_dbus_object_manager_server_new(dbus_object_path);
>> 	gchar *s;
>> 	s = g_strdup_printf("%s/%s",dbus_object_path,instance_name);
>> diff --git a/objects/hwmons_barreleye.c b/objects/hwmons_barreleye.c
>> index 06b1554..b9dfff6 100644
>> --- a/objects/hwmons_barreleye.c
>> +++ b/objects/hwmons_barreleye.c
>> @@ -1,5 +1,6 @@
>> #include "interfaces/openbmc_intf.h"
>> #include <stdio.h>
>> +#include <stdlib.h>
>> #include <fcntl.h>
>> #include "openbmc.h"
>> #include "gpio.h"
>> @@ -70,7 +71,7 @@ poll_hwmon(gpointer user_data)
>> 
>> 			}
>> 			guint32 value = atoi(buf)/scale;
>> -			GVariant* v = NEW_VARIANT_U(value);
>> +			(void)NEW_VARIANT_U(value);
>> 			GVariant* old_value = sensor_value_get_value(sensor);
>> 			bool do_set = false;
>> 			if(old_value == NULL)
>> @@ -130,8 +131,6 @@ on_bus_acquired(GDBusConnection *connection,
>> {
>> 	ObjectSkeleton *object;
>> 
>> -	cmdline *cmd = user_data;
>> -
>> 	manager = g_dbus_object_manager_server_new(dbus_object_path);
>> 	int i = 0;
>> 	for(i=0;i<NUM_HWMONS;i++)
>> diff --git a/objects/led_controller.c b/objects/led_controller.c
>> index 1f542e1..96e7ad1 100644
>> --- a/objects/led_controller.c
>> +++ b/objects/led_controller.c
>> @@ -5,6 +5,11 @@
>> #include <dirent.h>
>> #include <systemd/sd-bus.h>
>> 
>> +static int led_stable_state_function(const char *, const char *);
>> +static int led_default_blink(const char *, const char *);
>> +static int read_led(const char *, const char *, void *, const size_t);
>> +static int led_custom_blink(const char *, sd_bus_message *);
>> +
>> /*
>>  * These are control files that are present for each led under
>>  *'/sys/class/leds/<led_name>/' which are used to trigger action
>> @@ -156,8 +161,8 @@ led_function_router(sd_bus_message *msg, void *user_data,
>>  * Turn On or Turn Off the LED
>>  * --------------------------------------------------------------
>>  */
>> -int
>> -led_stable_state_function(char *led_name, char *led_function)
>> +static int
>> +led_stable_state_function(const char *led_name, const char *led_function)
>> {
>> 	/* Generic error reporter. */
>> 	int rc = -1;
>> @@ -244,8 +249,8 @@ blink_led(const char *led_name, const char *on_duration, const char *off_duratio
>>  * Default blink action on the LED.
>>  * ----------------------------------------------------
>>  */
>> -int
>> -led_default_blink(char *led_name, char *blink_type)
>> +static int
>> +led_default_blink(const char *led_name, const char *blink_type)
>> {
>> 	/* Generic error reporter */
>> 	int rc = -1;
>> @@ -281,7 +286,7 @@ led_default_blink(char *led_name, char *blink_type)
>>  * Blinks at user defined 'on' and 'off' intervals.
>>  * -------------------------------------------------
>>  */
>> -int
>> +static int
>> led_custom_blink(const char *led_name, sd_bus_message *msg)
>> {
>> 	/* Generic error reporter. */
>> @@ -342,7 +347,7 @@ led_custom_blink(const char *led_name, sd_bus_message *msg)
>>  * size -or- entire contents of file whichever is smaller
>>  * ----------------------------------------------------------------
>>  */
>> -int
>> +static int
>> read_led(const char *name, const char *ctrl_file,
>> 		void *value, const size_t len)
>> {
>> diff --git a/objects/pcie_slot_present_obj.c b/objects/pcie_slot_present_obj.c
>> index a71ba2d..0c9258e 100644
>> --- a/objects/pcie_slot_present_obj.c
>> +++ b/objects/pcie_slot_present_obj.c
>> @@ -87,7 +87,6 @@ update_fru_obj(GDBusConnection* connection, object_info* obj_info, const char* p
>> 	GDBusProxy *proxy;
>> 	GError *error;
>> 	GVariant *parm;
>> -	GVariant *result;
>> 
>> 	error = NULL;
>> 	proxy = g_dbus_proxy_new_sync(connection,
>> @@ -103,7 +102,7 @@ update_fru_obj(GDBusConnection* connection, object_info* obj_info, const char* p
>> 	error = NULL;
>> 	parm = g_variant_new("(s)",present);
>> 
>> -	result = g_dbus_proxy_call_sync(proxy,
>> +	(void)g_dbus_proxy_call_sync(proxy,
>> 			"setPresent",
>> 			parm,
>> 			G_DBUS_CALL_FLAGS_NONE,
>> @@ -121,8 +120,6 @@ main(gint argc, gchar *argv[])
>> 	GDBusConnection *c;
>> 	GDBusProxy *sys_proxy;
>> 	GError *error;
>> -	GVariant *parm;
>> -	GVariant *result;
>> 
>> 	loop = g_main_loop_new(NULL, FALSE);
>> 
>> @@ -146,7 +143,6 @@ main(gint argc, gchar *argv[])
>> 	{
>> 		object_info obj_info;
>> 		uint8_t present;
>> -		char* chr_present;
>> 		do {
>> 			rc = get_object(sys_proxy,&slots[i],&obj_info);
>> 			if(rc) { break; }
>> diff --git a/objects/power_control_obj.c b/objects/power_control_obj.c
>> index 72947f5..da69413 100644
>> --- a/objects/power_control_obj.c
>> +++ b/objects/power_control_obj.c
>> @@ -36,8 +36,6 @@ poll_pgood(gpointer user_data)
>> 	Control* control = object_get_control((Object*)user_data);
>> 
>> 	//send the heartbeat
>> -	const gchar* obj_path = g_dbus_object_get_object_path((GDBusObject*)user_data);
>> -
>> 	guint poll_int = control_get_poll_interval(control);
>> 	if(poll_int == 0)
>> 	{
>> @@ -119,7 +117,6 @@ on_set_power_state(ControlPower *pwr,
>> 		gpointer user_data)
>> {
>> 	Control* control = object_get_control((Object*)user_data);
>> -	const gchar* obj_path = g_dbus_object_get_object_path((GDBusObject*)user_data);
>> 	if(state > 1)
>> 	{
>> 		g_dbus_method_invocation_return_dbus_error(invocation,
> 
> _______________________________________________
> openbmc mailing list
> openbmc at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/openbmc


More information about the openbmc mailing list