Staging: csr: csr_msgconv: Fixed Multiple Coding Style Issues

PATCH In review Jake Champlin 2 participants 3 messages Jan 26, 2013 Staging

lore

Share & tools

lore

Jake Champlin

author

2013-01-26 20:14 UTC #0 lore raw

PATCH 1/1 Staging: csr: csr_msgconv: Fixed Multiple Coding Style Issues

Resolved 150 Style Errors and 202 Coding Style Warnings.

Signed-off-by Jake Champlin <[email protected]>

---
 drivers/staging/csr/csr_msgconv.c | 391 ++++++++++++++++----------------------
 1 file changed, 168 insertions(+), 223 deletions(-)

drivers/staging/csr/csr_msgconv.c +168 ~~-223~~

index db5e845..4f96edb 100644
@@ -1,10 +1,10 @@
 /*****************************************************************************
 
-            (c) Cambridge Silicon Radio Limited 2010
-            All rights reserved and confidential information of CSR
+	(c) Cambridge Silicon Radio Limited 2010
+	All rights reserved and confidential information of CSR
 
-            Refer to LICENSE.txt included with this source for details
-            on the license terms.
+	Refer to LICENSE.txt included with this source for details
+	on the license terms.
 
 *****************************************************************************/

@@ -19,273 +19,218 @@ static CsrMsgConvEntry *converter;
 
 CsrMsgConvPrimEntry *CsrMsgConvFind(u16 primType)
 {
-    CsrMsgConvPrimEntry *ptr = NULL;
-
-    if (converter)
-    {
-        ptr = converter->profile_converters;
-        while (ptr)
-        {
-            if (ptr->primType == primType)
-            {
-                break;
-            }
-            else
-            {
-                ptr = ptr->next;
-            }
-        }
-    }
-
-    return ptr;
+	CsrMsgConvPrimEntry *ptr = NULL;
+
+	if (converter) {
+		ptr = converter->profile_converters;
+	while (ptr) {
+		if (ptr->primType == primType)
+			break;
+		else
+			ptr = ptr->next;
+	}
+	}
+
+	return ptr;
 }
 
-static const CsrMsgConvMsgEntry *find_msg_converter(CsrMsgConvPrimEntry *ptr, u16 msgType)
+static const CsrMsgConvMsgEntry *find_msg_converter(CsrMsgConvPrimEntry *ptr,
+		u16 msgType)
 {
-    const CsrMsgConvMsgEntry *cv = ptr->conv;
-    if (ptr->lookupFunc)
-    {
-        return (const CsrMsgConvMsgEntry *) ptr->lookupFunc((CsrMsgConvMsgEntry *) cv, msgType);
-    }
-
-    while (cv)
-    {
-        if (cv->serFunc == NULL)
-        {
-            /* We've reached the end of the chain */
-            cv = NULL;
-            break;
-        }
-
-        if (cv->msgType == msgType)
-        {
-            break;
-        }
-        else
-        {
-            cv++;
-        }
-    }
-
-    return cv;
+	const CsrMsgConvMsgEntry *cv = ptr->conv;
+	if (ptr->lookupFunc)
+		return (const CsrMsgConvMsgEntry *)
+			ptr->lookupFunc((CsrMsgConvMsgEntry *) cv, msgType);
+
+	while (cv) {
+		if (cv->serFunc == NULL) {
+			/* We've reached the end of the chain */
+			cv = NULL;
+			break;
+		}
+
+		if (cv->msgType == msgType)
+			break;
+		else
+			cv++;
+	}
+
+	return cv;
 }
 
 static void *deserialize_data(u16 primType,
-    size_t length,
-    u8 *data)
+	size_t length,
+	u8 *data)
 {
-    CsrMsgConvPrimEntry *ptr;
-    u8 *ret;
-
-    ptr = CsrMsgConvFind(primType);
-
-    if (ptr)
-    {
-        const CsrMsgConvMsgEntry *cv;
-        u16 msgId = 0;
-        size_t offset = 0;
-        CsrUint16Des(&msgId, data, &offset);
-
-        cv = find_msg_converter(ptr, msgId);
-        if (cv)
-        {
-            ret = cv->deserFunc(data, length);
-        }
-        else
-        {
-            ret = NULL;
-        }
-    }
-    else
-    {
-        ret = NULL;
-    }
-
-    return ret;
+	CsrMsgConvPrimEntry *ptr;
+	u8 *ret;
+
+	ptr = CsrMsgConvFind(primType);
+
+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = 0;
+		size_t offset = 0;
+		CsrUint16Des(&msgId, data, &offset);
+
+		cv = find_msg_converter(ptr, msgId);
+		if (cv)
+			ret = cv->deserFunc(data, length);
+		else
+			ret = NULL;
+	} else
+		ret = NULL;
+
+	return ret;
 }
 
 static size_t sizeof_message(u16 primType, void *msg)
 {
-    CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
-    size_t ret;
-
-    if (ptr)
-    {
-        const CsrMsgConvMsgEntry *cv;
-        u16 msgId = *(u16 *) msg;
-
-        cv = find_msg_converter(ptr, msgId);
-        if (cv)
-        {
-            ret = cv->sizeofFunc(msg);
-        }
-        else
-        {
-            ret = 0;
-        }
-    }
-    else
-    {
-        ret = 0;
-    }
-
-    return ret;
+	CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
+	size_t ret;
+
+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = *(u16 *) msg;
+
+		cv = find_msg_converter(ptr, msgId);
+		if (cv)
+			ret = cv->sizeofFunc(msg);
+		else
+			ret = 0;
+	} else
+		ret = 0;
+
+	return ret;
 }
 
 static u8 free_message(u16 primType, u8 *data)
 {
-    CsrMsgConvPrimEntry *ptr;
-    u8 ret;
-
-    ptr = CsrMsgConvFind(primType);
-
-    if (ptr)
-    {
-        const CsrMsgConvMsgEntry *cv;
-        u16 msgId = *(u16 *) data;
-
-        cv = find_msg_converter(ptr, msgId);
-        if (cv)
-        {
-            cv->freeFunc(data);
-            ret = TRUE;
-        }
-        else
-        {
-            ret = FALSE;
-        }
-    }
-    else
-    {
-        ret = FALSE;
-    }
-
-    return ret;
+	CsrMsgConvPrimEntry *ptr;
+	u8 ret;
+
+	ptr = CsrMsgConvFind(primType);
+
+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = *(u16 *) data;
+
+		cv = find_msg_converter(ptr, msgId);
+	if (cv) {
+		cv->freeFunc(data);
+		ret = TRUE;
+	} else
+		ret = FALSE;
+	} else
+		ret = FALSE;
+
+	return ret;
 }
 
 static u8 *serialize_message(u16 primType,
-    void *msg,
-    size_t *length,
-    u8 *buffer)
+	void *msg,
+	size_t *length,
+	u8 *buffer)
 {
-    CsrMsgConvPrimEntry *ptr;
-    u8 *ret;
-
-    ptr = CsrMsgConvFind(primType);
-
-    *length = 0;
-
-    if (ptr)
-    {
-        const CsrMsgConvMsgEntry *cv;
-
-        cv = find_msg_converter(ptr, *(u16 *) msg);
-        if (cv)
-        {
-            ret = cv->serFunc(buffer, length, msg);
-        }
-        else
-        {
-            ret = NULL;
-        }
-    }
-    else
-    {
-        ret = NULL;
-    }
-
-    return ret;
+	CsrMsgConvPrimEntry *ptr;
+	u8 *ret;
+
+	ptr = CsrMsgConvFind(primType);
+
+	*length = 0;
+
+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+
+		cv = find_msg_converter(ptr, *(u16 *) msg);
+		if (cv)
+			ret = cv->serFunc(buffer, length, msg);
+		else
+			ret = NULL;
+	} else
+		ret = NULL;
+
+	return ret;
 }
 
 size_t CsrMsgConvSizeof(u16 primType, void *msg)
 {
-    return sizeof_message(primType, msg);
+	return sizeof_message(primType, msg);
 }
 
-u8 *CsrMsgConvSerialize(u8 *buffer, size_t maxBufferOffset, size_t *offset, u16 primType, void *msg)
+u8 *CsrMsgConvSerialize(u8 *buffer, size_t maxBufferOffset,
+		size_t *offset, u16 primType, void *msg)
 {
-    if (converter)
-    {
-        size_t serializedLength;
-        u8 *bufSerialized;
-        u8 *bufOffset = &buffer[*offset];
-        bufSerialized = converter->serialize_message(primType, msg, &serializedLength, bufOffset);
-        *offset += serializedLength;
-        return bufSerialized;
-    }
-    else
-    {
-        return NULL;
-    }
+	if (converter) {
+		size_t serializedLength;
+		u8 *bufSerialized;
+		u8 *bufOffset = &buffer[*offset];
+		bufSerialized = converter->serialize_message(primType, msg,
+			&serializedLength, bufOffset);
+		*offset += serializedLength;
+		return bufSerialized;
+	} else
+		return NULL;
 }
 
 /* Insert profile converter at head of converter list. */
 void CsrMsgConvInsert(u16 primType, const CsrMsgConvMsgEntry *ce)
 {
-    CsrMsgConvPrimEntry *pc;
-    pc = CsrMsgConvFind(primType);
-
-    if (pc)
-    {
-        /* Already registered. Do nothing */
-    }
-    else
-    {
-        pc = kmalloc(sizeof(*pc), GFP_KERNEL);
-        pc->primType = primType;
-        pc->conv = ce;
-        pc->lookupFunc = NULL;
-        pc->next = converter->profile_converters;
-        converter->profile_converters = pc;
-    }
+	CsrMsgConvPrimEntry *pc;
+	pc = CsrMsgConvFind(primType);
+
+	if (pc) {
+		/* Already registered. Do nothing */
+	} else {
+		pc = kmalloc(sizeof(*pc), GFP_KERNEL);
+		pc->primType = primType;
+		pc->conv = ce;
+		pc->lookupFunc = NULL;
+		pc->next = converter->profile_converters;
+		converter->profile_converters = pc;
+	}
 }
 EXPORT_SYMBOL_GPL(CsrMsgConvInsert);
 
 CsrMsgConvMsgEntry *CsrMsgConvFindEntry(u16 primType, u16 msgType)
 {
-    CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
-    if (ptr)
-    {
-        return (CsrMsgConvMsgEntry *) find_msg_converter(ptr, msgType);
-    }
-    return NULL;
+	CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
+	if (ptr)
+		return (CsrMsgConvMsgEntry *) find_msg_converter(ptr, msgType);
+	return NULL;
 }
 EXPORT_SYMBOL_GPL(CsrMsgConvFindEntry);
 
 CsrMsgConvMsgEntry *CsrMsgConvFindEntryByMsg(u16 primType, const void *msg)
 {
-    CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
-    if (ptr && msg)
-    {
-        u16 msgType = *((u16 *) msg);
-        return (CsrMsgConvMsgEntry *) find_msg_converter(ptr, msgType);
-    }
-    return NULL;
+	CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
+	if (ptr && msg) {
+		u16 msgType = *((u16 *) msg);
+		return (CsrMsgConvMsgEntry *) find_msg_converter(ptr, msgType);
+	}
+	return NULL;
 }
 
-void CsrMsgConvCustomLookupRegister(u16 primType, CsrMsgCustomLookupFunc *lookupFunc)
+void CsrMsgConvCustomLookupRegister(u16 primType,
+	CsrMsgCustomLookupFunc *lookupFunc)
 {
-    CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
-    if (ptr)
-    {
-        ptr->lookupFunc = lookupFunc;
-    }
+	CsrMsgConvPrimEntry *ptr = CsrMsgConvFind(primType);
+	if (ptr)
+		ptr->lookupFunc = lookupFunc;
 }
 EXPORT_SYMBOL_GPL(CsrMsgConvCustomLookupRegister);
 
 CsrMsgConvEntry *CsrMsgConvInit(void)
 {
-    if (!converter)
-    {
-        converter = kmalloc(sizeof(CsrMsgConvEntry), GFP_KERNEL);
-
-        converter->profile_converters = NULL;
-        converter->free_message = free_message;
-        converter->sizeof_message = sizeof_message;
-        converter->serialize_message = serialize_message;
-        converter->deserialize_data = deserialize_data;
-    }
-
-    return converter;
+	if (!converter) {
+		converter = kmalloc(sizeof(CsrMsgConvEntry), GFP_KERNEL);
+		converter->profile_converters = NULL;
+		converter->free_message = free_message;
+		converter->sizeof_message = sizeof_message;
+		converter->serialize_message = serialize_message;
+		converter->deserialize_data = deserialize_data;
+	}
+
+	return converter;
 }
 EXPORT_SYMBOL_GPL(CsrMsgConvInit);

Signature · 7 lines

1.8.1.1


-- 
Jake Champlin
[email protected]
<MuttClient>

Dan Carpenter

2013-01-26 20:35 UTC #1 lore raw

Quoted 2 lines · Jake Champlin

Resolved 150 Style Errors and 202 Coding Style Warnings.

Normally we would prefer that these are broken up into a several different patches that each fix one type of style issue.

[patch 1/x] csr: csr_msgconv: use tab indents
[patch 2/x] csr: csr_msgconv: put curly braces on the right line etc...

drivers/staging/csr/csr_msgconv.c static CsrMsgConvEntry *converter; · line 32

Quoted 11 lines

drivers/staging/csr/csr_msgconv.c +11 -0

+	CsrMsgConvPrimEntry *ptr = NULL;
+
+	if (converter) {
+		ptr = converter->profile_converters;
+	while (ptr) {
+		if (ptr->primType == primType)
+			break;
+		else
+			ptr = ptr->next;
+	}
+	}

The while loop should be indented one more time.

Quoted 4 lines

drivers/staging/csr/csr_msgconv.c +4 -0

+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = 0;
+		size_t offset = 0;

drivers/staging/csr/csr_msgconv.c static CsrMsgConvEntry *converter; · line 83

Quoted 10 lines

drivers/staging/csr/csr_msgconv.c +10 -0

+		CsrUint16Des(&msgId, data, &offset);
+
+		cv = find_msg_converter(ptr, msgId);
+		if (cv)
+			ret = cv->deserFunc(data, length);
+		else
+			ret = NULL;
+	} else
+		ret = NULL;
+

It should be:

+
+ ret = NULL;
+ } else {
+ ret = NULL;
+ }

The rules on braces are a little bit complicated. If it's one line then don't use braces. If it's two lines and you don't *need* to use braces then still use braces.

	if (foo) {
		/* some comment */
		frob();
	}

If either side of the if else statement uses braces then use braces on both.

drivers/staging/csr/csr_msgconv.c static CsrMsgConvEntry *converter; · line 126

Quoted 13 lines

drivers/staging/csr/csr_msgconv.c +13 -0

+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = *(u16 *) data;
+
+		cv = find_msg_converter(ptr, msgId);
+	if (cv) {
+		cv->freeFunc(data);
+		ret = TRUE;
+	} else
+		ret = FALSE;
+	} else
+		ret = FALSE;
+

regards,
dan carpenter

[email protected]

author

2013-01-26 20:47 UTC #2 lore raw

Dan,
Ok, thank you.

Will fix errors and resubmit.

Signature · 3 lines

Jake Champlin
[email protected]
<MuttClient>

Quoted 79 lines · Dan Carpenter

Quoted 2 lines · Jake Champlin
Resolved 150 Style Errors and 202 Coding Style Warnings.
Normally we would prefer that these are broken up into a several different patches that each fix one type of style issue.
[patch 1/x] csr: csr_msgconv: use tab indents
[patch 2/x] csr: csr_msgconv: put curly braces on the right line etc...
drivers/staging/csr/csr_msgconv.c static CsrMsgConvEntry *converter; · line 32
Quoted 11 lines
drivers/staging/csr/csr_msgconv.c +11 -0
+	CsrMsgConvPrimEntry *ptr = NULL;
+
+	if (converter) {
+		ptr = converter->profile_converters;
+	while (ptr) {
+		if (ptr->primType == primType)
+			break;
+		else
+			ptr = ptr->next;
+	}
+	}
The while loop should be indented one more time.
Quoted 4 lines
drivers/staging/csr/csr_msgconv.c +4 -0
+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = 0;
+		size_t offset = 0;
drivers/staging/csr/csr_msgconv.c static CsrMsgConvEntry *converter; · line 83
Quoted 10 lines
drivers/staging/csr/csr_msgconv.c +10 -0
+		CsrUint16Des(&msgId, data, &offset);
+
+		cv = find_msg_converter(ptr, msgId);
+		if (cv)
+			ret = cv->deserFunc(data, length);
+		else
+			ret = NULL;
+	} else
+		ret = NULL;
+
It should be:
drivers/staging/csr/csr_msgconv.c +5 -0
+
+			ret = NULL;
+	} else {
+		ret = NULL;
+	}
The rules on braces are a little bit complicated. If it's one line then don't use braces. If it's two lines and you don't *need* to use braces then still use braces.
	if (foo) {
		/* some comment */
		frob();
	}
If either side of the if else statement uses braces then use braces on both.
drivers/staging/csr/csr_msgconv.c static CsrMsgConvEntry *converter; · line 126
Quoted 13 lines
drivers/staging/csr/csr_msgconv.c +13 -0
+	if (ptr) {
+		const CsrMsgConvMsgEntry *cv;
+		u16 msgId = *(u16 *) data;
+
+		cv = find_msg_converter(ptr, msgId);
+	if (cv) {
+		cv->freeFunc(data);
+		ret = TRUE;
+	} else
+		ret = FALSE;
+	} else
+		ret = FALSE;
+
??
regards,
dan carpenter