Hello.

On 01/02/2014 07:48 PM, Arnd Bergmann wrote:

quoted

quoted

diff --git a/drivers/isdn/divert/divert_procfs.c b/drivers/isdn/divert/divert_procfs.c
index fb4f1ba..1c5dc34 100644
--- a/drivers/isdn/divert/divert_procfs.c
+++ b/drivers/isdn/divert/divert_procfs.c

@@ -86,12 +86,13 @@ isdn_divert_read(struct file *file, char __user *buf, size_t count, loff_t *off)
       struct divert_info *inf;
       int len;

-     if (!*((struct divert_info **) file->private_data)) {
+     if (!(inf = *((struct divert_info **) file->private_data))) {

quoted

     checkpatch.pl shouldn't approve assignment inside *if*. Though you're
moving it from the existing code, it wouldn't hurt to fix it.

I tried to touch as little as possible, and while I wouldn't use that
style myself, it is applied consistently in this driver, including the
wait_event line I'm adding, where I feel it actually makes sense.

    I wasn't feeling sure about that one. It seems to me now that it doesn't 
make much sense -- we could do the assignment beforehand.

quoted

quoted

               if (file->f_flags & O_NONBLOCK)
                       return -EAGAIN;
-             interruptible_sleep_on(&(rd_queue));
+             wait_event_interruptible(rd_queue, (inf =
+                     *((struct divert_info **) file->private_data)));

quoted

     Parens around assignment are hardly useful.

We get a gcc warning without them:

drivers/isdn/divert/divert_procfs.c:95:14: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
     *((struct divert_info **) file->private_data));

    Ah...

I can still change the first one (in both files) if you think it's important,

    I always prefer checkpatch.pl-clean patches, though some people do argue 
when they are just moving the badly styled code around.

but I'd rather not spend too much energy at coding style changes.

    Let's wait for the maintainer's opinion on this.

	Arnd

WBR, Sergei