DSP Bridge bunch of fixes

PATCH In review Andy Shevchenko 2 participants 19 messages Aug 27, 2009 – Sep 1, 2009

Share & tools

Andy Shevchenko

author

2009-08-27 12:19 UTC #0 lore raw

Hello.

Here are several fixes related to DSP Bridge driver for TI OMAP. Please, review them and include to the mainline.
Thank you.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

author

2009-08-27 12:19 UTC #1 lore raw

PATCH 1/8 dspbridge: Remove useless check

From: Andy Shevchenko <[email protected]>

Before this check zlLib already dereferenced on one hand and DBC_Require() checks for NULL on other hand.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/pmgr/dbll.c |   34 ++++++++++++++++------------------
 1 files changed, 16 insertions(+), 18 deletions(-)

drivers/dsp/bridge/pmgr/dbll.c +16 ~~-18~~

index 7c3cd40..e655f89 100644
@@ -909,26 +909,24 @@ void DBLL_unload(struct DBLL_LibraryObj *lib, struct DBLL_Attrs *attrs)
 		goto func_end;
 
 	zlLib->pTarget->attrs = *attrs;
-	if (zlLib != NULL) {
-		if (zlLib->mHandle) {
-			err = Dynamic_Unload_Module(zlLib->mHandle,
-				&zlLib->symbol.dlSymbol,
-				&zlLib->allocate.dlAlloc, &zlLib->init.dlInit);
-			if (err != 0) {
-				GT_1trace(DBLL_debugMask, GT_5CLASS,
-					 "Dynamic_Unload_Module "
-					 "failed: 0x%x\n", err);
-			}
-		}
-		/* remove symbols from symbol table */
-		if (zlLib->symTab != NULL) {
-			GH_delete(zlLib->symTab);
-			zlLib->symTab = NULL;
+	if (zlLib->mHandle) {
+		err = Dynamic_Unload_Module(zlLib->mHandle,
+			&zlLib->symbol.dlSymbol,
+			&zlLib->allocate.dlAlloc, &zlLib->init.dlInit);
+		if (err != 0) {
+			GT_1trace(DBLL_debugMask, GT_5CLASS,
+				 "Dynamic_Unload_Module "
+				 "failed: 0x%x\n", err);
 		}
-		/* delete DOFF desc since it holds *lots* of host OS
-		 * resources */
-		dofClose(zlLib);
 	}
+	/* remove symbols from symbol table */
+	if (zlLib->symTab != NULL) {
+		GH_delete(zlLib->symTab);
+		zlLib->symTab = NULL;
+	}
+	/* delete DOFF desc since it holds *lots* of host OS
+	 * resources */
+	dofClose(zlLib);
 func_end:
 	DBC_Ensure(zlLib->loadRef >= 0);
 }

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #2 lore raw

PATCH 2/8 dspbridge: Drop useless memory allocation

From: Andy Shevchenko <[email protected]>

strcmp() should do the job without additional memory allocation and strncpy()/strcmp() calls.

Additionally fix spelling.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/node.c |   15 ++++-----------
 1 files changed, 4 insertions(+), 11 deletions(-)

drivers/dsp/bridge/rmgr/node.c +4 ~~-11~~

index d3f0e34..e213b22 100644
@@ -404,7 +404,6 @@ DSP_STATUS NODE_Allocate(struct PROC_OBJECT *hProcessor,
 	DSP_STATUS status = DSP_SOK;
 	struct CMM_OBJECT *hCmmMgr = NULL; /* Shared memory manager hndl */
 	u32 procId;
-	char *label;
 	u32 pulValue;
 	u32 dynextBase;
 	u32 offSet = 0;

@@ -691,18 +690,16 @@ func_cont2:
 		}
 	}
 
-	/* Comapare value read from Node Properties and check if it is same as
+	/* Compare value read from Node Properties and check if it is same as
 	 * STACKSEGLABEL, if yes read the Address of STACKSEGLABEL, calculate
 	 * GPP Address, Read the value in that address and override the
 	 * uStackSeg value in task args */
 	if (DSP_SUCCEEDED(status) &&
 	   (char *)pNode->dcdProps.objData.nodeObj.ndbProps.uStackSegName !=
 	   NULL) {
-		label = MEM_Calloc(sizeof(STACKSEGLABEL)+1, MEM_PAGED);
-               strncpy(label, STACKSEGLABEL, sizeof(STACKSEGLABEL)+1);
-
-               if (strcmp((char *)pNode->dcdProps.objData.nodeObj.
-				     ndbProps.uStackSegName, label) == 0) {
+		if (strcmp((char *)
+		    pNode->dcdProps.objData.nodeObj.ndbProps.uStackSegName,
+		    STACKSEGLABEL) == 0) {
 			status = hNodeMgr->nldrFxns.pfnGetFxnAddr(pNode->
 				 hNldrNode, "DYNEXT_BEG", &dynextBase);
 			if (DSP_FAILED(status)) {

@@ -744,10 +741,6 @@ func_cont2:
 				ulStackSegVal;
 
 		}
-
-		if (label)
-			MEM_Free(label);
-
 	}

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #3 lore raw

PATCH 3/8 DSPBRIDGE: Check input value

From: Andy Shevchenko <[email protected]>

Check input value before dereferencing it.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/wmd/tiomap3430.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

drivers/dsp/bridge/wmd/tiomap3430.c +6 -1

index db48c49..4cb78c7 100644
@@ -1266,11 +1266,16 @@ static DSP_STATUS WMD_DEV_Destroy(struct WMD_DEV_CONTEXT *hDevContext)
 	DSP_STATUS status = DSP_SOK;
 	struct WMD_DEV_CONTEXT *pDevContext = (struct WMD_DEV_CONTEXT *)
 						hDevContext;
+
+	/* It should never happen */
+	if (!hDevContext)
+		return DSP_EHANDLE;
+
 	DBG_Trace(DBG_ENTER, "Entering WMD_DEV_Destroy:n hDevContext ::0x%x\n",
 		  hDevContext);
 	/* first put the device to stop state */
 	WMD_BRD_Delete(pDevContext);
-	if (pDevContext && pDevContext->pPtAttrs) {
+	if (pDevContext->pPtAttrs) {
 		pPtAttrs = pDevContext->pPtAttrs;
 		if (pPtAttrs->hCSObj)
 			SYNC_DeleteCS(pPtAttrs->hCSObj);

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #4 lore raw

PATCH 4/8 dspbridge: Check pointer returned by MEM_Calloc()

From: Andy Shevchenko <[email protected]>

In case of NULL return DSP_EMEMORY status instead of DSP_SOK.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/drv.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +13 ~~-10~~

index 691c727..bedc34c 100644
@@ -1571,17 +1571,18 @@ static DSP_STATUS RequestBridgeResources(u32 dwContext, s32 bRequest)
 		/* Releasing resources by deleting the registry key  */
 		dwBuffSize = sizeof(struct CFG_HOSTRES);
 		pResources = MEM_Calloc(dwBuffSize, MEM_NONPAGED);
-		if (DSP_FAILED(REG_GetValue(NULL, (char *)driverExt->szString,
-		   CURRENTCONFIG, (u8 *)pResources, &dwBuffSize))) {
-			status = CFG_E_RESOURCENOTAVAIL;
-			GT_0trace(curTrace, GT_1CLASS,
-				 "REG_GetValue Failed \n");
-		} else {
-			GT_0trace(curTrace, GT_1CLASS,
-				 "REG_GetValue Succeeded \n");
-		}
-
 		if (pResources != NULL) {
+			if (DSP_FAILED(REG_GetValue(NULL,
+			    (char *) driverExt->szString,
+			    CURRENTCONFIG, (u8 *) pResources, &dwBuffSize))) {
+				status = CFG_E_RESOURCENOTAVAIL;
+				GT_0trace(curTrace, GT_1CLASS,
+					 "REG_GetValue Failed \n");
+			} else {
+				GT_0trace(curTrace, GT_1CLASS,
+					 "REG_GetValue Succeeded \n");
+			}
+
 			dwBuffSize = sizeof(shm_size);
 			status = REG_GetValue(NULL, CURRENTCONFIG, SHMSIZE,
 				(u8 *)&shm_size, &dwBuffSize);

@@ -1646,6 +1647,8 @@ static DSP_STATUS RequestBridgeResources(u32 dwContext, s32 bRequest)
 				 (u32)dwBuffSize);
 			/*  Set all the other entries to NULL */
 			MEM_Free(pResources);
+		} else {
+			status = DSP_EMEMORY;
 		}
 		GT_0trace(curTrace, GT_ENTER, " <- RequestBridgeResources \n");
 		return status;

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #5 lore raw

PATCH 5/8 dspbridge: Check pointer before dereferencing it

From: Andy Shevchenko <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/nldr.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

drivers/dsp/bridge/rmgr/nldr.c +14 -1

index 79f7505..8c162a1 100644
@@ -1560,6 +1560,13 @@ static DSP_STATUS LoadOvly(struct NLDR_NODEOBJECT *hNldrNode,
 	}
 
 	DBC_Assert(i < hNldr->nOvlyNodes);
+
+	if (!pONode) {
+		/* Should we print warning here? */
+		status = DSP_ENOTFOUND;
+		goto func_end;
+	}
+
 	switch (phase) {
 	case NLDR_CREATE:
 		pRefCount = &(pONode->createRef);

@@ -1877,6 +1884,13 @@ static void UnloadOvly(struct NLDR_NODEOBJECT *hNldrNode, enum NLDR_PHASE phase)
 	}
 
 	DBC_Assert(i < hNldr->nOvlyNodes);
+
+	if (!pONode) {
+		/* Should we print warning here? */
+		status = DSP_ENOTFOUND;
+		goto func_end;
+	}
+
 	switch (phase) {
 	case NLDR_CREATE:
 		pRefCount = &(pONode->createRef);

@@ -1917,7 +1931,6 @@ static void UnloadOvly(struct NLDR_NODEOBJECT *hNldrNode, enum NLDR_PHASE phase)
 	}
 	if (pOtherRef && *pOtherRef == 0)
 		FreeSects(hNldr, pOtherSects, nOtherAlloc);
-
 }
 
 /*

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #6 lore raw

PATCH 6/8 dspbridge: Check status before use return value

From: Andy Shevchenko <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/drv.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +12 -2

index bedc34c..44d7f2d 100644
@@ -825,7 +825,13 @@ DSP_STATUS DRV_ProcDisplayResInfo(u8 *pBuf1, u32 *pSize)
 	u32 tempStrLen = 0, tempStrLen2 = 0;
 	DSP_STATUS status = DSP_SOK;
 
-	CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT);
+	if (DSP_FAILED(CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT))) {
+		/* Should we print something? */
+		*pBuf1 = 0;
+		*pSize = 0;
+		return DSP_EHANDLE;
+	}
+
 	DRV_GetProcCtxtList(&pCtxt, (struct DRV_OBJECT *)hDrvObject);
 	GT_0trace(curTrace, GT_ENTER, "*********************"
 		 "DRV_ProcDisplayResourceInfo:*\n");

@@ -954,8 +960,12 @@ static DSP_STATUS PrintProcessInformation(void)
 	u32 tempCount;
 	u32  procID;
 
+	if (DSP_FAILED(CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT))) {
+		/* Should we print something? */
+		return DSP_EHANDLE;
+	}
+
 	/* Get the Process context list */
-	CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT);
 	DRV_GetProcCtxtList(&pCtxtList, hDrvObject);
 	GT_0trace(curTrace, GT_4CLASS, "\n### Debug information"
 			" for DSP bridge ##\n");

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #7 lore raw

PATCH 7/8 DSPBRIDGE: Check return value

From: Andy Shevchenko <[email protected]>

PROC_GetProcessorId() potentially could return different to DSP_SOK status. We need to check it.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/drv.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +6 -2

index 44d7f2d..d21071c 100644
@@ -990,8 +990,12 @@ static DSP_STATUS PrintProcessInformation(void)
 		spin_lock(&pCtxtList->proc_list_lock);
 		list_for_each_entry(proc_obj_ptr, &pCtxtList->processor_list,
 				proc_object) {
-			PROC_GetProcessorId(proc_obj_ptr, &procID);
-			if (procID == DSP_UNIT) {
+			DSP_STATUS status2 = PROC_GetProcessorId(proc_obj_ptr,
+								 &procID);
+			if (DSP_FAILED(status2)) {
+				GT_0trace(curTrace, GT_7CLASS, "\n***ERROR:"
+					  "Unable to get Processor Id***\n");
+			} else if (procID == DSP_UNIT) {
 				GT_0trace(curTrace, GT_4CLASS,
 					"\nProcess connected to"
 					" DSP Processor\n");

Signature · 1 line

1.5.6.5

Andy Shevchenko

author

2009-08-27 12:19 UTC #8 lore raw

PATCH 8/8 dspbridge: Check pointer before usage

From: Andy Shevchenko <[email protected]>

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/drv.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +4 -0

index d21071c..491dd39 100644
@@ -1545,6 +1545,10 @@ DSP_STATUS DRV_ReleaseResources(u32 dwContext, struct DRV_OBJECT *hDrvObject)
 	for (pszdevNode = (struct DRV_EXT *)DRV_GetFirstDevExtension();
 	    pszdevNode != NULL; pszdevNode = (struct DRV_EXT *)
 	    DRV_GetNextDevExtension((u32)pszdevNode)) {
+		if (!pDRVObject->devNodeString) {
+			/* When this could happen? */
+			continue;
+		}
 		if ((u32)pszdevNode == dwContext) {
 			/* Found it */
 			/* Delete from the Driver object list */

Signature · 1 line

1.5.6.5

Guzman Lugo, Fernando

2009-08-31 20:47 UTC #9 lore raw

RE: [PATCH 8/8] dspbridge: Check pointer before usage

Hi,

Please see my comments below.

Quoted 20 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 8/8] dspbridge: Check pointer before usage
From: Andy Shevchenko <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/rmgr/drv.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
drivers/dsp/bridge/rmgr/drv.c +0 -0
index d21071c..491dd39 100644

drivers/dsp/bridge/rmgr/drv.c DSP_STATUS DRV_ReleaseResources(u32 dwContext, · line 1552

Quoted 8 lines

drivers/dsp/bridge/rmgr/drv.c +4 -0

@@ -1545,6 +1545,10 @@ DSP_STATUS DRV_ReleaseResources(u32 dwContext,
 struct DRV_OBJECT *hDrvObject)
 	for (pszdevNode = (struct DRV_EXT *)DRV_GetFirstDevExtension();
 	    pszdevNode != NULL; pszdevNode = (struct DRV_EXT *)
 	    DRV_GetNextDevExtension((u32)pszdevNode)) {
+		if (!pDRVObject->devNodeString) {
+			/* When this could happen? */
+			continue;
+		}

The function DSP_STATUS DRV_ReleaseResources is only called after call DRV_Create where the list is created:
...
/* Create and Initialize List of device Extension */

			pDRVObject->devNodeString = LST_Create();

...
If the list creation fails DRV_ReleaseResources is never call neither. Even if it was possible pDRVObject->devNodeString is not changing between "for" iterations so if you put a "continue" it will enter to the "for" loop and check again pDRVObject->devNodeString what will be still null, so I think is should be a "break" instead of "continue". What do you think?

Quoted 10 lines

drivers/dsp/bridge/rmgr/drv.c +0 -0
 		if ((u32)pszdevNode == dwContext) {
 			/* Found it */
			/* Delete from the Driver object list */
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Regards,
Fernando.

Andy Shevchenko

author

2009-09-01 08:00 UTC #10 lore raw

PATCH 1/1 dspbridge: Check pointer before usage

From: Andy Shevchenko <[email protected]>

Theoretically list could be deleted before we go into the for loop. Additionally we could delete list immediately if it becomes empty.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/drv.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +7 -2

index d21071c..9f726a6 100644
@@ -1545,18 +1545,23 @@ DSP_STATUS DRV_ReleaseResources(u32 dwContext, struct DRV_OBJECT *hDrvObject)
 	for (pszdevNode = (struct DRV_EXT *)DRV_GetFirstDevExtension();
 	    pszdevNode != NULL; pszdevNode = (struct DRV_EXT *)
 	    DRV_GetNextDevExtension((u32)pszdevNode)) {
+		if (!pDRVObject->devNodeString)
+			/* List already deleted */
+			break;
+
 		if ((u32)pszdevNode == dwContext) {
 			/* Found it */
 			/* Delete from the Driver object list */
 			LST_RemoveElem(pDRVObject->devNodeString,
-				      (struct LST_ELEM *)pszdevNode);
+				       (struct LST_ELEM *) pszdevNode);
 			MEM_Free((void *) pszdevNode);
-			break;
 		}
+
 		/* Delete the List if it is empty */
 		if (LST_IsEmpty(pDRVObject->devNodeString)) {
 			LST_Delete(pDRVObject->devNodeString);
 			pDRVObject->devNodeString = NULL;
+			break;
 		}
 	}
 	return status;

Signature · 1 line

1.5.6.5

[email protected]

author

2009-09-01 08:11 UTC #11 lore raw

PATCH 1/1 dspbridge: Check pointer before usage

From: Andy Shevchenko <[email protected]>

Check list pointer at every itteration because it could be deleted before we go into the for loop.

Reworked version of the previous attempts.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/dsp/bridge/rmgr/drv.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +3 -2

index d21071c..7e41669 100644
@@ -1543,8 +1543,9 @@ DSP_STATUS DRV_ReleaseResources(u32 dwContext, struct DRV_OBJECT *hDrvObject)
 	 *  The following will over write the status.
 	 */
 	for (pszdevNode = (struct DRV_EXT *)DRV_GetFirstDevExtension();
-	    pszdevNode != NULL; pszdevNode = (struct DRV_EXT *)
-	    DRV_GetNextDevExtension((u32)pszdevNode)) {
+	    pszdevNode != NULL && pDRVObject->devNodeString != NULL;
+	    pszdevNode = (struct DRV_EXT *)
+			  DRV_GetNextDevExtension((u32) pszdevNode)) {
 		if ((u32)pszdevNode == dwContext) {
 			/* Found it */
 			/* Delete from the Driver object list */

Signature · 1 line

1.5.6.5

Guzman Lugo, Fernando

Acked-by on v1 7/8

2009-08-31 20:21 UTC #12 lore raw

RE: [PATCH 7/8] DSPBRIDGE: Check return value

In reply to Andy Shevchenko

Acked-by Fernando Guzman Lugo <[email protected]>

Quoted 45 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 7/8] DSPBRIDGE: Check return value
From: Andy Shevchenko <[email protected]>
PROC_GetProcessorId() potentially could return different to DSP_SOK status. We
need to check it.
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/rmgr/drv.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
drivers/dsp/bridge/rmgr/drv.c +6 -3
index 44d7f2d..d21071c 100644
@@ -990,8 +990,12 @@ static DSP_STATUS PrintProcessInformation(void)
 		spin_lock(&pCtxtList->proc_list_lock);
 		list_for_each_entry(proc_obj_ptr, &pCtxtList->processor_list,
 				proc_object) {
-			PROC_GetProcessorId(proc_obj_ptr, &procID);
-			if (procID == DSP_UNIT) {
+			DSP_STATUS status2 = PROC_GetProcessorId(proc_obj_ptr,
+								 &procID);
+			if (DSP_FAILED(status2)) {
+				GT_0trace(curTrace, GT_7CLASS, "\n***ERROR:"
+					  "Unable to get Processor Id***\n");
+			} else if (procID == DSP_UNIT) {
 				GT_0trace(curTrace, GT_4CLASS,
 					"\nProcess connected to"
 					" DSP Processor\n");
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guzman Lugo, Fernando

Acked-by on v1 6/8

2009-08-31 20:21 UTC #13 lore raw

RE: [PATCH 6/8] dspbridge: Check status before use return value

In reply to Andy Shevchenko

Acked-by Fernando Guzman Lugo <[email protected]>

Quoted 56 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 6/8] dspbridge: Check status before use return value
From: Andy Shevchenko <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/rmgr/drv.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
drivers/dsp/bridge/rmgr/drv.c +7 -1
index bedc34c..44d7f2d 100644
@@ -825,7 +825,13 @@ DSP_STATUS DRV_ProcDisplayResInfo(u8 *pBuf1, u32
 *pSize)
 	u32 tempStrLen = 0, tempStrLen2 = 0;
 	DSP_STATUS status = DSP_SOK;
 
-	CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT);
+	if (DSP_FAILED(CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT))) {
+		/* Should we print something? */
+		*pBuf1 = 0;
+		*pSize = 0;
+		return DSP_EHANDLE;
+	}
+
 	DRV_GetProcCtxtList(&pCtxt, (struct DRV_OBJECT *)hDrvObject);
 	GT_0trace(curTrace, GT_ENTER, "*********************"
		 "DRV_ProcDisplayResourceInfo:*\n");
drivers/dsp/bridge/rmgr/drv.c +5 -2
@@ -954,8 +960,12 @@ static DSP_STATUS PrintProcessInformation(void)
 	u32 tempCount;
 	u32  procID;
 
+	if (DSP_FAILED(CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT))) {
+		/* Should we print something? */
+		return DSP_EHANDLE;
+	}
+
 	/* Get the Process context list */
-	CFG_GetObject((u32 *)&hDrvObject, REG_DRV_OBJECT);
 	DRV_GetProcCtxtList(&pCtxtList, hDrvObject);
 	GT_0trace(curTrace, GT_4CLASS, "\n### Debug information"
 			" for DSP bridge ##\n");
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guzman Lugo, Fernando

Acked-by on v1 5/8

2009-08-31 20:20 UTC #14 lore raw

RE: [PATCH 5/8] dspbridge: Check pointer before dereferencing it

In reply to Andy Shevchenko

It looks good.

Acked-by Fernando Guzman Lugo <[email protected]>

Quoted 66 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 5/8] dspbridge: Check pointer before dereferencing it
From: Andy Shevchenko <[email protected]>
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/rmgr/nldr.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
drivers/dsp/bridge/rmgr/nldr.c +7 -0
index 79f7505..8c162a1 100644
@@ -1560,6 +1560,13 @@ static DSP_STATUS LoadOvly(struct NLDR_NODEOBJECT
 *hNldrNode,
 	}
 
 	DBC_Assert(i < hNldr->nOvlyNodes);
+
+	if (!pONode) {
+		/* Should we print warning here? */
+		status = DSP_ENOTFOUND;
+		goto func_end;
+	}
+
 	switch (phase) {
 	case NLDR_CREATE:
		pRefCount = &(pONode->createRef);
drivers/dsp/bridge/rmgr/nldr.c +7 -0
@@ -1877,6 +1884,13 @@ static void UnloadOvly(struct NLDR_NODEOBJECT
 *hNldrNode, enum NLDR_PHASE phase)
 	}
 
 	DBC_Assert(i < hNldr->nOvlyNodes);
+
+	if (!pONode) {
+		/* Should we print warning here? */
+		status = DSP_ENOTFOUND;
+		goto func_end;
+	}
+
 	switch (phase) {
 	case NLDR_CREATE:
		pRefCount = &(pONode->createRef);
drivers/dsp/bridge/rmgr/nldr.c +0 -1
@@ -1917,7 +1931,6 @@ static void UnloadOvly(struct NLDR_NODEOBJECT
 *hNldrNode, enum NLDR_PHASE phase)
 	}
 	if (pOtherRef && *pOtherRef == 0)
 		FreeSects(hNldr, pOtherSects, nOtherAlloc);
-
 }
 
/*
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guzman Lugo, Fernando

Acked-by on v1 4/8

2009-08-31 20:19 UTC #15 lore raw

RE: [PATCH 4/8] dspbridge: Check pointer returned by MEM_Calloc()

In reply to Andy Shevchenko

Acked-by Fernando Guzman Lugo <[email protected]>

Quoted 68 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 4/8] dspbridge: Check pointer returned by MEM_Calloc()

From: Andy Shevchenko <[email protected]>

In case of NULL return DSP_EMEMORY status instead of DSP_SOK.

Signed-off-by Andy Shevchenko <[email protected]>

---
drivers/dsp/bridge/rmgr/drv.c | 23 +++++++++++++----------

1 files changed, 13 insertions(+), 10 deletions(-)

drivers/dsp/bridge/rmgr/drv.c +11 ~~-10~~

index 691c727..bedc34c 100644
@@ -1571,17 +1571,18 @@ static DSP_STATUS RequestBridgeResources(u32
 dwContext, s32 bRequest)
 		/* Releasing resources by deleting the registry key  */
 		dwBuffSize = sizeof(struct CFG_HOSTRES);
 		pResources = MEM_Calloc(dwBuffSize, MEM_NONPAGED);
-		if (DSP_FAILED(REG_GetValue(NULL, (char *)driverExt->szString,
-		   CURRENTCONFIG, (u8 *)pResources, &dwBuffSize))) {
-			status = CFG_E_RESOURCENOTAVAIL;
-			GT_0trace(curTrace, GT_1CLASS,
-				 "REG_GetValue Failed \n");
-		} else {
-			GT_0trace(curTrace, GT_1CLASS,
-				 "REG_GetValue Succeeded \n");
-		}
-
 		if (pResources != NULL) {
+			if (DSP_FAILED(REG_GetValue(NULL,
+			    (char *) driverExt->szString,
+			    CURRENTCONFIG, (u8 *) pResources, &dwBuffSize))) {
+				status = CFG_E_RESOURCENOTAVAIL;
+				GT_0trace(curTrace, GT_1CLASS,
+					 "REG_GetValue Failed \n");
+			} else {
+				GT_0trace(curTrace, GT_1CLASS,
+					 "REG_GetValue Succeeded \n");
+			}
+
 			dwBuffSize = sizeof(shm_size);
 			status = REG_GetValue(NULL, CURRENTCONFIG, SHMSIZE,

				(u8 *)&shm_size, &dwBuffSize);

drivers/dsp/bridge/rmgr/drv.c +2 -0

@@ -1646,6 +1647,8 @@ static DSP_STATUS RequestBridgeResources(u32
 dwContext, s32 bRequest)
 				 (u32)dwBuffSize);
 			/*  Set all the other entries to NULL */
 			MEM_Free(pResources);
+		} else {
+			status = DSP_EMEMORY;
 		}
 		GT_0trace(curTrace, GT_ENTER, " <- RequestBridgeResources \n");

		return status;

--
1.5.6.5

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guzman Lugo, Fernando

2009-08-31 20:14 UTC #16 lore raw

RE: [PATCH 3/8] DSPBRIDGE: Check input value

In reply to Andy Shevchenko

Hi,

	Good finding.

	Acked-by: Fernando Guzman Lugo <[email protected]>

Quoted 49 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 3/8] DSPBRIDGE: Check input value
From: Andy Shevchenko <[email protected]>
Check input value before dereferencing it.
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/wmd/tiomap3430.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
drivers/dsp/bridge/wmd/tiomap3430.c +6 -1
index db48c49..4cb78c7 100644
@@ -1266,11 +1266,16 @@ static DSP_STATUS WMD_DEV_Destroy(struct
 WMD_DEV_CONTEXT *hDevContext)
 	DSP_STATUS status = DSP_SOK;
 	struct WMD_DEV_CONTEXT *pDevContext = (struct WMD_DEV_CONTEXT *)
 						hDevContext;
+
+	/* It should never happen */
+	if (!hDevContext)
+		return DSP_EHANDLE;
+
 	DBG_Trace(DBG_ENTER, "Entering WMD_DEV_Destroy:n hDevContext
 ::0x%x\n",
 		  hDevContext);
 	/* first put the device to stop state */
 	WMD_BRD_Delete(pDevContext);
-	if (pDevContext && pDevContext->pPtAttrs) {
+	if (pDevContext->pPtAttrs) {
 		pPtAttrs = pDevContext->pPtAttrs;
		if (pPtAttrs->hCSObj)
			SYNC_DeleteCS(pPtAttrs->hCSObj);
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guzman Lugo, Fernando

Acked-by on v1 2/8

2009-08-31 20:49 UTC #17 lore raw

RE: [PATCH 2/8] dspbridge: Drop useless memory allocation

In reply to Andy Shevchenko

Hi,
Good finding.

Acked-by Fernando Guzman Lugo <[email protected]>

Quoted 76 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 2/8] dspbridge: Drop useless memory allocation
From: Andy Shevchenko <[email protected]>
strcmp() should do the job without additional memory allocation and strncpy()/strcmp() calls.
Additionally fix spelling.
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/rmgr/node.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
drivers/dsp/bridge/rmgr/node.c +0 -1
index d3f0e34..e213b22 100644
@@ -404,7 +404,6 @@ DSP_STATUS NODE_Allocate(struct PROC_OBJECT
 *hProcessor,
 	DSP_STATUS status = DSP_SOK;
 	struct CMM_OBJECT *hCmmMgr = NULL; /* Shared memory manager hndl */
 	u32 procId;
-	char *label;
 	u32 pulValue;
 	u32 dynextBase;
	u32 offSet = 0;
drivers/dsp/bridge/rmgr/node.c +0 -1
@@ -691,18 +690,16 @@ func_cont2:
 		}
 	}
 
-	/* Comapare value read from Node Properties and check if it is same
as
drivers/dsp/bridge/rmgr/node.c +1 -0
+	/* Compare value read from Node Properties and check if it is same as
	 * STACKSEGLABEL, if yes read the Address of STACKSEGLABEL, calculate
	 * GPP Address, Read the value in that address and override the
	 * uStackSeg value in task args */
	if (DSP_SUCCEEDED(status) &&
	   (char *)pNode->dcdProps.objData.nodeObj.ndbProps.uStackSegName !=
	   NULL) {
drivers/dsp/bridge/rmgr/node.c +3 -5
-		label = MEM_Calloc(sizeof(STACKSEGLABEL)+1, MEM_PAGED);
-               strncpy(label, STACKSEGLABEL, sizeof(STACKSEGLABEL)+1);
-
-               if (strcmp((char *)pNode->dcdProps.objData.nodeObj.
-				     ndbProps.uStackSegName, label) == 0) {
+		if (strcmp((char *)
+		    pNode->dcdProps.objData.nodeObj.ndbProps.uStackSegName,
+		    STACKSEGLABEL) == 0) {
			status = hNodeMgr->nldrFxns.pfnGetFxnAddr(pNode->
				 hNldrNode, "DYNEXT_BEG", &dynextBase);
			if (DSP_FAILED(status)) {
drivers/dsp/bridge/rmgr/node.c +0 -5
@@ -744,10 +741,6 @@ func_cont2:
 				ulStackSegVal;
 
 		}
-
-		if (label)
-			MEM_Free(label);
-
 	}
 
 
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Guzman Lugo, Fernando

Acked-by on v1 1/8

2009-08-31 20:12 UTC #18 lore raw

RE: [PATCH 1/8] dspbridge: Remove useless check

In reply to Andy Shevchenko

Hi,
Looks good.

Acked-by Fernando Guzman Lugo <[email protected]>

Quoted 74 lines

-----Original Message-----
From: [email protected] [mailto:linux-omap- [email protected]] On Behalf Of Andy Shevchenko
Sent: Thursday, August 27, 2009 7:19 AM
To: [email protected]
Cc: Andy Shevchenko
Subject: [PATCH 1/8] dspbridge: Remove useless check
From: Andy Shevchenko <[email protected]>
Before this check zlLib already dereferenced on one hand and DBC_Require() checks for NULL on other hand.
Signed-off-by Andy Shevchenko <[email protected]>
---
drivers/dsp/bridge/pmgr/dbll.c | 34 ++++++++++++++++------------------
1 files changed, 16 insertions(+), 18 deletions(-)
drivers/dsp/bridge/pmgr/dbll.c +16 ~~-18~~
index 7c3cd40..e655f89 100644
@@ -909,26 +909,24 @@ void DBLL_unload(struct DBLL_LibraryObj *lib, struct
 DBLL_Attrs *attrs)
 		goto func_end;
 
 	zlLib->pTarget->attrs = *attrs;
-	if (zlLib != NULL) {
-		if (zlLib->mHandle) {
-			err = Dynamic_Unload_Module(zlLib->mHandle,
-				&zlLib->symbol.dlSymbol,
-				&zlLib->allocate.dlAlloc, &zlLib->init.dlInit);
-			if (err != 0) {
-				GT_1trace(DBLL_debugMask, GT_5CLASS,
-					 "Dynamic_Unload_Module "
-					 "failed: 0x%x\n", err);
-			}
-		}
-		/* remove symbols from symbol table */
-		if (zlLib->symTab != NULL) {
-			GH_delete(zlLib->symTab);
-			zlLib->symTab = NULL;
+	if (zlLib->mHandle) {
+		err = Dynamic_Unload_Module(zlLib->mHandle,
+			&zlLib->symbol.dlSymbol,
+			&zlLib->allocate.dlAlloc, &zlLib->init.dlInit);
+		if (err != 0) {
+			GT_1trace(DBLL_debugMask, GT_5CLASS,
+				 "Dynamic_Unload_Module "
+				 "failed: 0x%x\n", err);
 		}
-		/* delete DOFF desc since it holds *lots* of host OS
-		 * resources */
-		dofClose(zlLib);
 	}
+	/* remove symbols from symbol table */
+	if (zlLib->symTab != NULL) {
+		GH_delete(zlLib->symTab);
+		zlLib->symTab = NULL;
+	}
+	/* delete DOFF desc since it holds *lots* of host OS
+	 * resources */
+	dofClose(zlLib);
func_end:
	DBC_Ensure(zlLib->loadRef >= 0);
}
--
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html