public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic
@ 2015-09-28 16:21 Ilya Verbin
  2015-09-28 16:30 ` Jakub Jelinek
  0 siblings, 1 reply; 5+ messages in thread
From: Ilya Verbin @ 2015-09-28 16:21 UTC (permalink / raw)
  To: gcc-patches; +Cc: Kirill Yukhin, Jakub Jelinek

Committed to trunk as obvious.

	PR other/67652
	* runtime/offload_engine.cpp (Engine::init_process): Fix sizeof.

diff --git a/liboffloadmic/runtime/offload_engine.cpp b/liboffloadmic/runtime/offload_engine.cpp
index 16b440d..00b673a 100644
--- a/liboffloadmic/runtime/offload_engine.cpp
+++ b/liboffloadmic/runtime/offload_engine.cpp
@@ -173,7 +173,7 @@ void Engine::init_process(void)
             // use putenv instead of setenv as Windows has no setenv.
             // Note: putenv requires its argument can't be freed or modified.
             // So no free after call to putenv or elsewhere.
-            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2" + 1));
+            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2"));
             sprintf(env_var, "COI_DMA_CHANNEL_COUNT=2");
             putenv(env_var);  
         }

  -- Ilya

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic
  2015-09-28 16:21 [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic Ilya Verbin
@ 2015-09-28 16:30 ` Jakub Jelinek
  2015-09-28 16:32   ` Ilya Verbin
  2015-09-28 16:50   ` Andrew Pinski
  0 siblings, 2 replies; 5+ messages in thread
From: Jakub Jelinek @ 2015-09-28 16:30 UTC (permalink / raw)
  To: Ilya Verbin; +Cc: gcc-patches, Kirill Yukhin

On Mon, Sep 28, 2015 at 07:10:13PM +0300, Ilya Verbin wrote:
> Committed to trunk as obvious.
> 
> 	PR other/67652
> 	* runtime/offload_engine.cpp (Engine::init_process): Fix sizeof.
> 
> diff --git a/liboffloadmic/runtime/offload_engine.cpp b/liboffloadmic/runtime/offload_engine.cpp
> index 16b440d..00b673a 100644
> --- a/liboffloadmic/runtime/offload_engine.cpp
> +++ b/liboffloadmic/runtime/offload_engine.cpp
> @@ -173,7 +173,7 @@ void Engine::init_process(void)
>              // use putenv instead of setenv as Windows has no setenv.
>              // Note: putenv requires its argument can't be freed or modified.
>              // So no free after call to putenv or elsewhere.
> -            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2" + 1));
> +            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2"));
>              sprintf(env_var, "COI_DMA_CHANNEL_COUNT=2");
>              putenv(env_var);  

Missing error handling if malloc returns NULL?

	Jakub

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic
  2015-09-28 16:30 ` Jakub Jelinek
@ 2015-09-28 16:32   ` Ilya Verbin
  2015-09-28 16:50   ` Andrew Pinski
  1 sibling, 0 replies; 5+ messages in thread
From: Ilya Verbin @ 2015-09-28 16:32 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches, Kirill Yukhin

On Mon, Sep 28, 2015 at 18:15:14 +0200, Jakub Jelinek wrote:
> On Mon, Sep 28, 2015 at 07:10:13PM +0300, Ilya Verbin wrote:
> > Committed to trunk as obvious.
> > 
> > 	PR other/67652
> > 	* runtime/offload_engine.cpp (Engine::init_process): Fix sizeof.
> > 
> > diff --git a/liboffloadmic/runtime/offload_engine.cpp b/liboffloadmic/runtime/offload_engine.cpp
> > index 16b440d..00b673a 100644
> > --- a/liboffloadmic/runtime/offload_engine.cpp
> > +++ b/liboffloadmic/runtime/offload_engine.cpp
> > @@ -173,7 +173,7 @@ void Engine::init_process(void)
> >              // use putenv instead of setenv as Windows has no setenv.
> >              // Note: putenv requires its argument can't be freed or modified.
> >              // So no free after call to putenv or elsewhere.
> > -            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2" + 1));
> > +            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2"));
> >              sprintf(env_var, "COI_DMA_CHANNEL_COUNT=2");
> >              putenv(env_var);  
> 
> Missing error handling if malloc returns NULL?

Yes :(
I will grep all mallocs/reallocs one more time.

  -- Ilya

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic
  2015-09-28 16:30 ` Jakub Jelinek
  2015-09-28 16:32   ` Ilya Verbin
@ 2015-09-28 16:50   ` Andrew Pinski
  2015-10-08 19:09     ` Ilya Verbin
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Pinski @ 2015-09-28 16:50 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Ilya Verbin, GCC Patches, Kirill Yukhin

On Mon, Sep 28, 2015 at 9:15 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Sep 28, 2015 at 07:10:13PM +0300, Ilya Verbin wrote:
>> Committed to trunk as obvious.
>>
>>       PR other/67652
>>       * runtime/offload_engine.cpp (Engine::init_process): Fix sizeof.
>>
>> diff --git a/liboffloadmic/runtime/offload_engine.cpp b/liboffloadmic/runtime/offload_engine.cpp
>> index 16b440d..00b673a 100644
>> --- a/liboffloadmic/runtime/offload_engine.cpp
>> +++ b/liboffloadmic/runtime/offload_engine.cpp
>> @@ -173,7 +173,7 @@ void Engine::init_process(void)
>>              // use putenv instead of setenv as Windows has no setenv.
>>              // Note: putenv requires its argument can't be freed or modified.
>>              // So no free after call to putenv or elsewhere.
>> -            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2" + 1));
>> +            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2"));
>>              sprintf(env_var, "COI_DMA_CHANNEL_COUNT=2");
>>              putenv(env_var);
>
> Missing error handling if malloc returns NULL?


Also why not just use strdup here? instead of malloc/sizeof/sprintf ?

Thanks,
Andrew Pinski

>
>         Jakub

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic
  2015-09-28 16:50   ` Andrew Pinski
@ 2015-10-08 19:09     ` Ilya Verbin
  0 siblings, 0 replies; 5+ messages in thread
From: Ilya Verbin @ 2015-10-08 19:09 UTC (permalink / raw)
  To: gcc-patches; +Cc: Andrew Pinski, Jakub Jelinek, Kirill Yukhin

On Mon, Sep 28, 2015 at 18:15:14 +0200, Jakub Jelinek wrote:
> > -            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2" + 1));
> > +            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2"));
> >              sprintf(env_var, "COI_DMA_CHANNEL_COUNT=2");
> >              putenv(env_var);  
> 
> Missing error handling if malloc returns NULL?

Fixed.

On Mon, Sep 28, 2015 at 09:19:30 -0700, Andrew Pinski wrote:
> Also why not just use strdup here? instead of malloc/sizeof/sprintf ?

Done.

Committed as obvious.


liboffloadmic/
	* runtime/offload_engine.cpp (Engine::init_process): Use strdup instead
	of sizeof+malloc+sprintf, check for return value.
	* runtime/offload_env.cpp (MicEnvVar::get_env_var_kind): Check for
	strdup return value.
	* runtime/offload_host.cpp (__offload_init_library_once): Check for
	strdup return value.  Fix size calculation of COI_HOST_THREAD_AFFINITY.
	* runtime/emulator/coi_device.cpp (COIProcessWaitForShutdown): Check for
	malloc return value.


diff --git a/liboffloadmic/runtime/offload_engine.cpp b/liboffloadmic/runtime/offload_engine.cpp
index 00b673a..4a88546 100644
--- a/liboffloadmic/runtime/offload_engine.cpp
+++ b/liboffloadmic/runtime/offload_engine.cpp
@@ -173,8 +173,9 @@ void Engine::init_process(void)
             // use putenv instead of setenv as Windows has no setenv.
             // Note: putenv requires its argument can't be freed or modified.
             // So no free after call to putenv or elsewhere.
-            char * env_var = (char*) malloc(sizeof("COI_DMA_CHANNEL_COUNT=2"));
-            sprintf(env_var, "COI_DMA_CHANNEL_COUNT=2");
+            char * env_var = strdup("COI_DMA_CHANNEL_COUNT=2");
+	    if (env_var == NULL)
+	      LIBOFFLOAD_ERROR(c_malloc);
             putenv(env_var);  
         }
     }
diff --git a/liboffloadmic/runtime/offload_env.cpp b/liboffloadmic/runtime/offload_env.cpp
index 79f5f36..ac33b67 100644
--- a/liboffloadmic/runtime/offload_env.cpp
+++ b/liboffloadmic/runtime/offload_env.cpp
@@ -212,10 +212,14 @@ MicEnvVarKind MicEnvVar::get_env_var_kind(
             *env_var_name_length = 3;
             *env_var_name = *env_var_def = c;
             *env_var_def = strdup(*env_var_def);
+	    if (*env_var_def == NULL)
+	      LIBOFFLOAD_ERROR(c_malloc);
             return  c_mic_var;
         }
         *env_var_def = c + strlen("ENV=");
         *env_var_def = strdup(*env_var_def);
+	if (*env_var_def == NULL)
+	  LIBOFFLOAD_ERROR(c_malloc);
         return c_mic_card_env;
     }
     if (isalpha(*c)) {
@@ -229,6 +233,8 @@ MicEnvVarKind MicEnvVar::get_env_var_kind(
         return c_no_mic;
     }
     *env_var_def = strdup(*env_var_def);
+    if (*env_var_def == NULL)
+      LIBOFFLOAD_ERROR(c_malloc);
     return card_is_set? c_mic_card_var : c_mic_var;
 }
 
diff --git a/liboffloadmic/runtime/offload_host.cpp b/liboffloadmic/runtime/offload_host.cpp
index 08f626f..eec457d 100644
--- a/liboffloadmic/runtime/offload_host.cpp
+++ b/liboffloadmic/runtime/offload_host.cpp
@@ -5173,6 +5173,8 @@ static void __offload_init_library_once(void)
         if (strcasecmp(env_var, "none") != 0) {
             // value is composed of comma separated physical device indexes
             char *buf = strdup(env_var);
+	    if (buf == NULL)
+	      LIBOFFLOAD_ERROR(c_malloc);
             char *str, *ptr;
             for (str = strtok_r(buf, ",", &ptr); str != 0;
                  str = strtok_r(0, ",", &ptr)) {
@@ -5245,7 +5247,9 @@ static void __offload_init_library_once(void)
     if (env_var != 0) {
         char * new_env_var =
                    (char*) malloc(sizeof("COI_HOST_THREAD_AFFINITY=") +
-                                  sizeof(env_var) + 1);
+                                  strlen(env_var));
+	if (new_env_var == NULL)
+	  LIBOFFLOAD_ERROR(c_malloc);
         sprintf(new_env_var, "COI_HOST_THREAD_AFFINITY=%s", env_var);
         putenv(new_env_var);
     }
@@ -5254,6 +5258,8 @@ static void __offload_init_library_once(void)
     env_var = getenv("MIC_LD_LIBRARY_PATH");
     if (env_var != 0) {
         mic_library_path = strdup(env_var);
+	if (mic_library_path == NULL)
+	  LIBOFFLOAD_ERROR(c_malloc);
     }
 
 
@@ -5262,6 +5268,8 @@ static void __offload_init_library_once(void)
     const char *base_name = "offload_main";
     if (mic_library_path != 0) {
         char *buf = strdup(mic_library_path);
+	if (buf == NULL)
+	  LIBOFFLOAD_ERROR(c_malloc);
         char *try_name = (char*) alloca(strlen(mic_library_path) +
                 strlen(base_name) + 2);
         char *dir, *ptr;
@@ -5275,6 +5283,8 @@ static void __offload_init_library_once(void)
             struct stat st;
             if (stat(try_name, &st) == 0 && S_ISREG(st.st_mode)) {
                 mic_device_main = strdup(try_name);
+		if (mic_device_main == NULL)
+		  LIBOFFLOAD_ERROR(c_malloc);
                 break;
             }
         }
@@ -5345,6 +5355,8 @@ static void __offload_init_library_once(void)
     env_var = getenv("MIC_PROXY_FS_ROOT");
     if (env_var != 0 && *env_var != '\0') {
         mic_proxy_fs_root = strdup(env_var);
+	if (mic_proxy_fs_root == NULL)
+	  LIBOFFLOAD_ERROR(c_malloc);
     }
 
     // Prepare environment for the target process using the following
diff --git a/liboffloadmic/runtime/emulator/coi_device.cpp b/liboffloadmic/runtime/emulator/coi_device.cpp
index f9dd2cd..983fec05 100644
--- a/liboffloadmic/runtime/emulator/coi_device.cpp
+++ b/liboffloadmic/runtime/emulator/coi_device.cpp
@@ -362,7 +362,8 @@ SYMBOL_VERSION (COIProcessWaitForShutdown, 1) ()
 	case CMD_PIPELINE_CREATE:
 	  {
 	    /* Receive data from host.  */
-	    uint32_t *pipeline_num = (uint32_t *) malloc (sizeof (uint32_t));
+	    uint32_t *pipeline_num;
+	    MALLOC (uint32_t *, pipeline_num, sizeof (uint32_t));
 	    READ (pipe_host2tgt, pipeline_num, sizeof (*pipeline_num));
 
 	    /* Create a new thread for the pipeline.  */


  -- Ilya

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-10-08 19:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-28 16:21 [PATCH][committed] Fix PR67652: wrong sizeof calculation in liboffloadmic Ilya Verbin
2015-09-28 16:30 ` Jakub Jelinek
2015-09-28 16:32   ` Ilya Verbin
2015-09-28 16:50   ` Andrew Pinski
2015-10-08 19:09     ` Ilya Verbin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).