diff mbox

linux-gen: _ishm: checking fstat return value.

Message ID 1484930402-41533-1-git-send-email-christophe.milard@linaro.org
State Superseded
Headers show

Commit Message

Christophe Milard Jan. 20, 2017, 4:40 p.m. UTC
Hence fixing CID 174663

Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

---
 platform/linux-generic/_ishm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.7.4

Comments

Mike Holmes Jan. 20, 2017, 3:53 p.m. UTC | #1
On 20 January 2017 at 11:40, Christophe Milard
<christophe.milard@linaro.org> wrote:
> Hence fixing CID 174663


Adding:

Fixes https://bugs.linaro.org/show_bug.cgi?id=2827

Makes it easy to track closing a bug

>

> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

> ---

>  platform/linux-generic/_ishm.c | 7 ++++++-

>  1 file changed, 6 insertions(+), 1 deletion(-)

>

> diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c

> index f889834..8da64a1 100644

> --- a/platform/linux-generic/_ishm.c

> +++ b/platform/linux-generic/_ishm.c

> @@ -818,7 +818,12 @@ int _odp_ishm_reserve(const char *name, uint64_t size, int fd,

>

>         /* If a file descriptor is provided, get the real size and map: */

>         if (fd >= 0) {

> -               fstat(fd, &statbuf);

> +               if (fstat(fd, &statbuf) < 0) {

> +                       close(fd);

> +                       odp_spinlock_unlock(&ishm_tbl->lock);

> +                       ODP_ERR("_ishm_reserve failed (fstat failed).\n");

> +                       return -1;

> +               }

>                 len = statbuf.st_size;

>                 /* note that the huge page flag is meningless here as huge

>                  * page is determined by the provided file descriptor: */

> --

> 2.7.4

>




-- 
Mike Holmes
Program Manager - Linaro Networking Group
Linaro.org │ Open source software for ARM SoCs
"Work should be fun and collaborative, the rest follows"
Bill Fischofer Jan. 21, 2017, 10:49 p.m. UTC | #2
Agree with Mike on adding the Bug to the commit log

On Fri, Jan 20, 2017 at 9:53 AM, Mike Holmes <mike.holmes@linaro.org> wrote:
> On 20 January 2017 at 11:40, Christophe Milard

> <christophe.milard@linaro.org> wrote:

>> Hence fixing CID 174663

>

> Adding:

>

> Fixes https://bugs.linaro.org/show_bug.cgi?id=2827

>

> Makes it easy to track closing a bug

>

>>

>> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>


Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

>> ---

>>  platform/linux-generic/_ishm.c | 7 ++++++-

>>  1 file changed, 6 insertions(+), 1 deletion(-)

>>

>> diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c

>> index f889834..8da64a1 100644

>> --- a/platform/linux-generic/_ishm.c

>> +++ b/platform/linux-generic/_ishm.c

>> @@ -818,7 +818,12 @@ int _odp_ishm_reserve(const char *name, uint64_t size, int fd,

>>

>>         /* If a file descriptor is provided, get the real size and map: */

>>         if (fd >= 0) {

>> -               fstat(fd, &statbuf);

>> +               if (fstat(fd, &statbuf) < 0) {

>> +                       close(fd);

>> +                       odp_spinlock_unlock(&ishm_tbl->lock);

>> +                       ODP_ERR("_ishm_reserve failed (fstat failed).\n");

>> +                       return -1;

>> +               }

>>                 len = statbuf.st_size;

>>                 /* note that the huge page flag is meningless here as huge

>>                  * page is determined by the provided file descriptor: */

>> --

>> 2.7.4

>>

>

>

>

> --

> Mike Holmes

> Program Manager - Linaro Networking Group

> Linaro.org │ Open source software for ARM SoCs

> "Work should be fun and collaborative, the rest follows"
Maxim Uvarov Jan. 22, 2017, 11:06 a.m. UTC | #3
On 01/22/17 01:49, Bill Fischofer wrote:
> Agree with Mike on adding the Bug to the commit log

> 

> On Fri, Jan 20, 2017 at 9:53 AM, Mike Holmes <mike.holmes@linaro.org> wrote:

>> On 20 January 2017 at 11:40, Christophe Milard

>> <christophe.milard@linaro.org> wrote:

>>> Hence fixing CID 174663

>>

>> Adding:

>>

>> Fixes https://bugs.linaro.org/show_bug.cgi?id=2827

>>

>> Makes it easy to track closing a bug

>>

>>>

>>> Signed-off-by: Christophe Milard <christophe.milard@linaro.org>

> 

> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

> 

>>> ---

>>>  platform/linux-generic/_ishm.c | 7 ++++++-

>>>  1 file changed, 6 insertions(+), 1 deletion(-)

>>>

>>> diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c

>>> index f889834..8da64a1 100644

>>> --- a/platform/linux-generic/_ishm.c

>>> +++ b/platform/linux-generic/_ishm.c

>>> @@ -818,7 +818,12 @@ int _odp_ishm_reserve(const char *name, uint64_t size, int fd,

>>>

>>>         /* If a file descriptor is provided, get the real size and map: */

>>>         if (fd >= 0) {

>>> -               fstat(fd, &statbuf);

>>> +               if (fstat(fd, &statbuf) < 0) {

>>> +                       close(fd);

>>> +                       odp_spinlock_unlock(&ishm_tbl->lock);

>>> +                       ODP_ERR("_ishm_reserve failed (fstat failed).\n");


it's better to print the reason why it failed.

it will be good to set __odp_errno here.


Maxim.



>>> +                       return -1;

>>> +               }

>>>                 len = statbuf.st_size;

>>>                 /* note that the huge page flag is meningless here as huge

>>>                  * page is determined by the provided file descriptor: */

>>> --

>>> 2.7.4

>>>

>>

>>

>>

>> --

>> Mike Holmes

>> Program Manager - Linaro Networking Group

>> Linaro.org │ Open source software for ARM SoCs

>> "Work should be fun and collaborative, the rest follows"
diff mbox

Patch

diff --git a/platform/linux-generic/_ishm.c b/platform/linux-generic/_ishm.c
index f889834..8da64a1 100644
--- a/platform/linux-generic/_ishm.c
+++ b/platform/linux-generic/_ishm.c
@@ -818,7 +818,12 @@  int _odp_ishm_reserve(const char *name, uint64_t size, int fd,
 
 	/* If a file descriptor is provided, get the real size and map: */
 	if (fd >= 0) {
-		fstat(fd, &statbuf);
+		if (fstat(fd, &statbuf) < 0) {
+			close(fd);
+			odp_spinlock_unlock(&ishm_tbl->lock);
+			ODP_ERR("_ishm_reserve failed (fstat failed).\n");
+			return -1;
+		}
 		len = statbuf.st_size;
 		/* note that the huge page flag is meningless here as huge
 		 * page is determined by the provided file descriptor: */