Skip to contentSkip to conversation

Staging: android: alarm: fixed quoted string split across lines

PATCH In review Nasir Abed 4 participants 10 messages May 20, 2012 – May 21, 2012 Staging

lore
Share & tools
lore

Nasir Abed

author
#0 lore raw

PATCH 1/1 Staging: android: alarm: fixed quoted string split across lines

Fixed a warning quoted string split across lines.

---
 drivers/staging/android/alarm.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/android/alarm.c +2 -2
index c68950b..4ea07bb 100644
@@ -302,8 +302,8 @@ int android_alarm_set_rtc(struct timespec new_time)
 	}
 	ret = rtc_set_time(alarm_rtc_dev, &rtc_new_rtc_time);
 	if (ret < 0)
-		pr_alarm(ERROR, "alarm_set_rtc: "
-			"Failed to set RTC, time will be lost on reboot\n");
+		pr_alarm(ERROR,
+			 "alarm_set_rtc: Failed to set RTC, time will be lost on reboot\n");
 err:
 	wake_unlock(&alarm_rtc_wake_lock);
 	mutex_unlock(&alarm_setrtc_mutex);
Signature · 1 line
1.7.9.5
#4 lore raw
In reply to Nasir Abed
Quoted 5 lines · Nasir Abed
drivers/staging/android/alarm.c +0 -0
index c68950b..4ea07bb 100644

drivers/staging/android/alarm.c int android_alarm_set_rtc(struct timespec new_time) · line 309

Quoted 10 lines
drivers/staging/android/alarm.c +2 -2
@@ -302,8 +302,8 @@ int android_alarm_set_rtc(struct timespec new_time)
 	}
 	ret = rtc_set_time(alarm_rtc_dev, &rtc_new_rtc_time);
 	if (ret < 0)
-		pr_alarm(ERROR, "alarm_set_rtc: "
-			"Failed to set RTC, time will be lost on reboot\n");
+		pr_alarm(ERROR,
+			 "alarm_set_rtc: Failed to set RTC, time will be lost on reboot\n");
 err:
 	wake_unlock(&alarm_rtc_wake_lock);
 	mutex_unlock(&alarm_setrtc_mutex);

Looks like this would benefit from using __func__ since it already doesn't represent the correct function name.

#5 lore raw
Quoted 12 lines · David Rientjes
Quoted 10 lines · Nasir Abed
drivers/staging/android/alarm.c +0 -2
index c68950b..4ea07bb 100644
@@ -302,8 +302,8 @@ int android_alarm_set_rtc(struct timespec new_time)
 	}
 	ret = rtc_set_time(alarm_rtc_dev, &rtc_new_rtc_time);
 	if (ret < 0)
-		pr_alarm(ERROR, "alarm_set_rtc: "
-			"Failed to set RTC, time will be lost on reboot\n");
Quoted 8 lines

drivers/staging/android/alarm.c int android_alarm_set_rtc(struct timespec new_time) · line 309

Quoted 5 lines
drivers/staging/android/alarm.c +2 -0
+		pr_alarm(ERROR,
+			 "alarm_set_rtc: Failed to set RTC, time will be lost on reboot\n");
 err:
 	wake_unlock(&alarm_rtc_wake_lock);
 	mutex_unlock(&alarm_setrtc_mutex);

Looks like this would benefit from using __func__ since it already doesn't represent the correct function name.

true.

Somewhat related, pr_alarm is a poor name choice.
It should probably called alarm_dbg.

o it's a duplicated #define
it should be in some common #include
o it's a debug test that emits at KERN_INFO

#6 lore raw

PATCH 1/1 Staging: android: alarm: Rename pr_alarm to alarm_dbg

Rename a macro to make it explicit it's for debugging.

Use %s: __func__ instead of embedding function names. Coalesce formats, align arguments.

---
 drivers/staging/android/alarm-dev.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)
drivers/staging/android/alarm-dev.c +17 -17
index 53ce6ec..32614d6 100644
@@ -29,16 +29,14 @@
 #define ANDROID_ALARM_PRINT_IO (1U << 1)
 #define ANDROID_ALARM_PRINT_INT (1U << 2)
 
-
 static int debug_mask = ANDROID_ALARM_PRINT_INFO;
 module_param_named(debug_mask, debug_mask, int, S_IRUGO | S_IWUSR | S_IWGRP);
 
-#define pr_alarm(debug_level_mask, args...) \
-	do { \
-		if (debug_mask & ANDROID_ALARM_PRINT_##debug_level_mask) { \
-			pr_info(args); \
-		} \
-	} while (0)
+#define alarm_dbg(debug_level_mask, fmt, ...)				\
+do {									\
+	if (debug_mask & ANDROID_ALARM_PRINT_##debug_level_mask)	\
+		pr_info(fmt, ##__VA_ARGS__);				\
+} while (0)
 
 #define ANDROID_ALARM_WAKEUP_MASK ( \
 	ANDROID_ALARM_RTC_WAKEUP_MASK | \
@@ -138,7 +136,7 @@ static long alarm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	switch (ANDROID_ALARM_BASE_CMD(cmd)) {
 	case ANDROID_ALARM_CLEAR(0):
 		spin_lock_irqsave(&alarm_slock, flags);
-		pr_alarm(IO, "alarm %d clear\n", alarm_type);
+		alarm_dbg(IO, "alarm %d clear\n", alarm_type);
 		devalarm_try_to_cancel(&alarms[alarm_type]);
 		if (alarm_pending) {
 			alarm_pending &= ~alarm_type_mask;
@@ -167,8 +165,9 @@ static long alarm_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 		}
 from_old_alarm_set:
 		spin_lock_irqsave(&alarm_slock, flags);
-		pr_alarm(IO, "alarm %d set %ld.%09ld\n", alarm_type,
-			new_alarm_time.tv_sec, new_alarm_time.tv_nsec);
+		alarm_dbg(IO, "alarm %d set %ld.%09ld\n",
+			  alarm_type,
+			  new_alarm_time.tv_sec, new_alarm_time.tv_nsec);
 		alarm_enabled |= alarm_type_mask;
 		devalarm_start(&alarms[alarm_type],
 			timespec_to_ktime(new_alarm_time));
@@ -179,7 +178,7 @@ from_old_alarm_set:
 		/* fall though */
 	case ANDROID_ALARM_WAIT:
 		spin_lock_irqsave(&alarm_slock, flags);
-		pr_alarm(IO, "alarm wait\n");
+		alarm_dbg(IO, "alarm wait\n");
 		if (!alarm_pending && wait_pending) {
 			__pm_relax(&alarm_wake_lock);
 			wait_pending = 0;
@@ -260,9 +259,10 @@ static int alarm_release(struct inode *inode, struct file *file)
 		for (i = 0; i < ANDROID_ALARM_TYPE_COUNT; i++) {
 			uint32_t alarm_type_mask = 1U << i;
 			if (alarm_enabled & alarm_type_mask) {
-				pr_alarm(INFO, "alarm_release: clear alarm, "
-					"pending %d\n",
-					!!(alarm_pending & alarm_type_mask));
+				alarm_dbg(INFO,
+					  "%s: clear alarm, pending %d\n",
+					  __func__,
+					  !!(alarm_pending & alarm_type_mask));
 				alarm_enabled &= ~alarm_type_mask;
 			}
 			spin_unlock_irqrestore(&alarm_slock, flags);
@@ -271,8 +271,8 @@ static int alarm_release(struct inode *inode, struct file *file)
 		}
 		if (alarm_pending | wait_pending) {
 			if (alarm_pending)
-				pr_alarm(INFO, "alarm_release: clear "
-					"pending alarms %x\n", alarm_pending);
+				alarm_dbg(INFO, "%s: clear pending alarms %x\n",
+					  __func__, alarm_pending);
 			__pm_relax(&alarm_wake_lock);
 			wait_pending = 0;
 			alarm_pending = 0;
@@ -288,7 +288,7 @@ static void devalarm_triggered(struct devalarm *alarm)
 	unsigned long flags;
 	uint32_t alarm_type_mask = 1U << alarm->type;
 
-	pr_alarm(INT, "devalarm_triggered type %d\n", alarm->type);
+	alarm_dbg(INT, "%s: type %d\n", __func__, alarm->type);
 	spin_lock_irqsave(&alarm_slock, flags);
 	if (alarm_enabled & alarm_type_mask) {
 		__pm_wakeup_event(&alarm_wake_lock, 5000); /* 5secs */
author
#8 lore raw

Re: [PATCH] Staging: android: alarm: Rename pr_alarm to alarm_dbg

Shouldn't:
[PATCH] Staging: android: alarm: Rename pr_alarm to alarm_dbg Be:
[PATCH] Staging: android: alarm-dev: Rename pr_alarm to alarm_dbg

Cheers,

Nasir Abed

Quoted 10 lines · David Rientjes
Quoted 6 lines · Joe Perches

Rename a macro to make it explicit it's for debugging.

Use %s: __func__ instead of embedding function names. Coalesce formats, align arguments.

#9 lore raw

Re: [PATCH] Staging: android: alarm: Rename pr_alarm to alarm_dbg

Quoted 5 lines · Nasir Abed

Shouldn't:
[PATCH] Staging: android: alarm: Rename pr_alarm to alarm_dbg Be:
[PATCH] Staging: android: alarm-dev: Rename pr_alarm to alarm_dbg

Or even better "Staging: android-alarm:" but nobody cares one way or the other. Stop bellyaching about nonsense things. :P

regards,
dan carpenter