Re: [Mesa-dev] [PATCH 1/3] clover: fix _logs string creation

2014-09-05 Thread Jan Vesely
On Mon, 2014-08-18 at 09:20 +0300, Francisco Jerez wrote:
> EdB  writes:
> 
> > compact::string is not \0 terminated.
> > size() need to be used for std::string creation
> > ---
> >  src/gallium/state_trackers/clover/core/program.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
> > b/src/gallium/state_trackers/clover/core/program.cpp
> > index e09c3aa..3f504d5 100644
> > --- a/src/gallium/state_trackers/clover/core/program.cpp
> > +++ b/src/gallium/state_trackers/clover/core/program.cpp
> > @@ -61,9 +61,9 @@ program::build(const ref_vector &devs, const char 
> > *opts) {
> >  dev.ir_target(), 
> > build_opts(dev),
> >  log));
> >  _binaries.insert({ &dev, module });
> > -_logs.insert({ &dev, std::string(log.c_str()) });
> > +_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
> >   } catch (const build_error &) {
> > -_logs.insert({ &dev, std::string(log.c_str()) });
> > +_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
> 
> Both of these should just be using the conversion operator.  See
> attachment.

I came up with an almost identical patch to your attachment.
you can add my
Reviewed-by: Jan Vesely 

> 
> >  throw;
> >   }
> >}
> > -- 
> > 2.0.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Jan Vesely 


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] clover: fix _logs string creation

2014-08-18 Thread EdB
On Monday, August 18, 2014 09:20:03 AM Francisco Jerez wrote:
> EdB  writes:
> > compact::string is not \0 terminated.
> > size() need to be used for std::string creation
> > ---
> > 
> >  src/gallium/state_trackers/clover/core/program.cpp | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp
> > b/src/gallium/state_trackers/clover/core/program.cpp index
> > e09c3aa..3f504d5 100644
> > --- a/src/gallium/state_trackers/clover/core/program.cpp
> > +++ b/src/gallium/state_trackers/clover/core/program.cpp
> > @@ -61,9 +61,9 @@ program::build(const ref_vector &devs, const
> > char *opts) {> 
> >  dev.ir_target(),
> >  build_opts(dev),
> >  log));
> >  
> >  _binaries.insert({ &dev, module });
> > 
> > -_logs.insert({ &dev, std::string(log.c_str()) });
> > +_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
> > 
> >   } catch (const build_error &) {
> > 
> > -_logs.insert({ &dev, std::string(log.c_str()) });
> > +_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
> 
> Both of these should just be using the conversion operator.  See
> attachment.

Agreed, I was highlighting the problem.
Yours is better.

Thanks

> 
> >  throw;
> >   
> >   }
> >
> >}
> > 
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] clover: fix _logs string creation

2014-08-17 Thread Francisco Jerez
EdB  writes:

> compact::string is not \0 terminated.
> size() need to be used for std::string creation
> ---
>  src/gallium/state_trackers/clover/core/program.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
> b/src/gallium/state_trackers/clover/core/program.cpp
> index e09c3aa..3f504d5 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -61,9 +61,9 @@ program::build(const ref_vector &devs, const char 
> *opts) {
>  dev.ir_target(), 
> build_opts(dev),
>  log));
>  _binaries.insert({ &dev, module });
> -_logs.insert({ &dev, std::string(log.c_str()) });
> +_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
>   } catch (const build_error &) {
> -_logs.insert({ &dev, std::string(log.c_str()) });
> +_logs.insert({ &dev, std::string(log.c_str(), log.size()) });

Both of these should just be using the conversion operator.  See
attachment.

>  throw;
>   }
>}
> -- 
> 2.0.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

From 3c2bec6d790e6aa38fb6d71cd495f281205ddf6c Mon Sep 17 00:00:00 2001
From: Francisco Jerez 
Date: Mon, 18 Aug 2014 09:05:25 +0300
Subject: [PATCH] clover: Use conversion operator to initialize build log from
 compat::string.

Fixes binary garbage in the compilation logs caused by
compat::string::c_str() not being null-terminated (which is a bug on
its own that will be fixed in another commit).

Reported-by: EdB 
---
 src/gallium/state_trackers/clover/core/program.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
index 30a1f0e..6c224db 100644
--- a/src/gallium/state_trackers/clover/core/program.cpp
+++ b/src/gallium/state_trackers/clover/core/program.cpp
@@ -61,9 +61,9 @@ program::build(const ref_vector &devs, const char *opts) {
 dev.ir_target(), build_opts(dev),
 log));
 _binaries.insert({ &dev, module });
-_logs.insert({ &dev, std::string(log.c_str()) });
+_logs.insert({ &dev, log });
  } catch (const build_error &) {
-_logs.insert({ &dev, std::string(log.c_str()) });
+_logs.insert({ &dev, log });
 throw;
  }
   }
-- 
2.0.4



pgpejMi7uG3oD.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/3] clover: fix _logs string creation

2014-08-07 Thread EdB
compact::string is not \0 terminated.
size() need to be used for std::string creation
---
 src/gallium/state_trackers/clover/core/program.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
b/src/gallium/state_trackers/clover/core/program.cpp
index e09c3aa..3f504d5 100644
--- a/src/gallium/state_trackers/clover/core/program.cpp
+++ b/src/gallium/state_trackers/clover/core/program.cpp
@@ -61,9 +61,9 @@ program::build(const ref_vector &devs, const char 
*opts) {
 dev.ir_target(), 
build_opts(dev),
 log));
 _binaries.insert({ &dev, module });
-_logs.insert({ &dev, std::string(log.c_str()) });
+_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
  } catch (const build_error &) {
-_logs.insert({ &dev, std::string(log.c_str()) });
+_logs.insert({ &dev, std::string(log.c_str(), log.size()) });
 throw;
  }
   }
-- 
2.0.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev