Re: [Mesa-dev] [PATCH] nv50: fix a SIGSEGV with piglit bin/gl-3.1-vao-broken-attrib
On Mon, Jul 6, 2015 at 7:55 PM, Dylan Baker wrote:
>> >> + } else
>> >> + if (!vb->buffer) {
>> >
>> > Should the else and if be on the same line?
>>
>> The general style elsewhere is to do it in this weird way. Can't say
>> I'm a big fan, but I prefer consistency.
>>
>> I'd happily take a change that undid that oddity.
>>
>
> Odd, okay.
Its one advantage is it allows one to easily remove clauses either
with #if or with comments on a line level without editing other lines.
However that's a pretty rare occurrence.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50: fix a SIGSEGV with piglit bin/gl-3.1-vao-broken-attrib
> >> + } else
> >> + if (!vb->buffer) {
> >
> > Should the else and if be on the same line?
>
> The general style elsewhere is to do it in this weird way. Can't say
> I'm a big fan, but I prefer consistency.
>
> I'd happily take a change that undid that oddity.
>
Odd, okay.
signature.asc
Description: Digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50: fix a SIGSEGV with piglit bin/gl-3.1-vao-broken-attrib
On Mon, Jul 6, 2015 at 7:50 PM, Dylan Baker wrote:
> On Mon, Jul 06, 2015 at 11:34:23PM +0200, Samuel Pitoiset wrote:
>> Before validating vertex arrays we need to check if a VBO is present.
>> Checking if vb->buffer is not NULL fixes the issue.
>>
>> Signed-off-by: Samuel Pitoiset
>> ---
>> src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> index 1fd33b8..3d200bd 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
>> @@ -382,6 +382,11 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
>>if (nv50->vbo_user & (1 << b)) {
>> address = addrs[b] + ve->pipe.src_offset;
>> limit = addrs[b] + limits[b];
>> + } else
>> + if (!vb->buffer) {
>
> Should the else and if be on the same line?
The general style elsewhere is to do it in this weird way. Can't say
I'm a big fan, but I prefer consistency.
I'd happily take a change that undid that oddity.
>
>> + BEGIN_NV04(push, NV50_3D(VERTEX_ARRAY_FETCH(i)), 1);
>> + PUSH_DATA (push, 0);
>> + continue;
>>} else {
>> struct nv04_resource *buf = nv04_resource(vb->buffer);
>> if (!(refd & (1 << b))) {
>> --
>> 2.4.5
>>
>> ___
>> 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
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50: fix a SIGSEGV with piglit bin/gl-3.1-vao-broken-attrib
On Mon, Jul 06, 2015 at 11:34:23PM +0200, Samuel Pitoiset wrote:
> Before validating vertex arrays we need to check if a VBO is present.
> Checking if vb->buffer is not NULL fixes the issue.
>
> Signed-off-by: Samuel Pitoiset
> ---
> src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> index 1fd33b8..3d200bd 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> @@ -382,6 +382,11 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
>if (nv50->vbo_user & (1 << b)) {
> address = addrs[b] + ve->pipe.src_offset;
> limit = addrs[b] + limits[b];
> + } else
> + if (!vb->buffer) {
Should the else and if be on the same line?
> + BEGIN_NV04(push, NV50_3D(VERTEX_ARRAY_FETCH(i)), 1);
> + PUSH_DATA (push, 0);
> + continue;
>} else {
> struct nv04_resource *buf = nv04_resource(vb->buffer);
> if (!(refd & (1 << b))) {
> --
> 2.4.5
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: Digital signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50: fix a SIGSEGV with piglit bin/gl-3.1-vao-broken-attrib
Reviewed-by: Ilia Mirkin
But please change the commit title to mention the actual effect, like
"avoid segfault with enabled but unbound vertex attrib" or something.
The piglit fix should be mentioned in the body of the message -- it's
a nice side-effect, but not really title-worthy.
Also:
Cc: mesa-sta...@lists.freedesktop.org
in the commit so that it gets cherry-picked into stable releases.
On Mon, Jul 6, 2015 at 5:34 PM, Samuel Pitoiset
wrote:
> Before validating vertex arrays we need to check if a VBO is present.
> Checking if vb->buffer is not NULL fixes the issue.
>
> Signed-off-by: Samuel Pitoiset
> ---
> src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> index 1fd33b8..3d200bd 100644
> --- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> +++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
> @@ -382,6 +382,11 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
>if (nv50->vbo_user & (1 << b)) {
> address = addrs[b] + ve->pipe.src_offset;
> limit = addrs[b] + limits[b];
> + } else
> + if (!vb->buffer) {
> + BEGIN_NV04(push, NV50_3D(VERTEX_ARRAY_FETCH(i)), 1);
> + PUSH_DATA (push, 0);
> + continue;
>} else {
> struct nv04_resource *buf = nv04_resource(vb->buffer);
> if (!(refd & (1 << b))) {
> --
> 2.4.5
>
> ___
> 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
[Mesa-dev] [PATCH] nv50: fix a SIGSEGV with piglit bin/gl-3.1-vao-broken-attrib
Before validating vertex arrays we need to check if a VBO is present.
Checking if vb->buffer is not NULL fixes the issue.
Signed-off-by: Samuel Pitoiset
---
src/gallium/drivers/nouveau/nv50/nv50_vbo.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
index 1fd33b8..3d200bd 100644
--- a/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
+++ b/src/gallium/drivers/nouveau/nv50/nv50_vbo.c
@@ -382,6 +382,11 @@ nv50_vertex_arrays_validate(struct nv50_context *nv50)
if (nv50->vbo_user & (1 << b)) {
address = addrs[b] + ve->pipe.src_offset;
limit = addrs[b] + limits[b];
+ } else
+ if (!vb->buffer) {
+ BEGIN_NV04(push, NV50_3D(VERTEX_ARRAY_FETCH(i)), 1);
+ PUSH_DATA (push, 0);
+ continue;
} else {
struct nv04_resource *buf = nv04_resource(vb->buffer);
if (!(refd & (1 << b))) {
--
2.4.5
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
