Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(286)

Issue 199069: Review: broken getmessage (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 1 month ago by larrygritz
Modified:
14 years, 1 month ago
Reviewers:
ckulla
CC:
dev-osl_imageworks.com, osl-dev_googlegroups.com
Base URL:
http://openshadinglanguage.googlecode.com/svn/trunk/
Visibility:
Public.

Description

In the same way as my last review for string ops, if getmessage had all uniform args but was called from within a varying conditional, it could fill in only the first value of the result.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -0 lines) Patch
src/liboslexec/opmessage.cpp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 3
larrygritz
14 years, 1 month ago (2010-02-03 01:30:25 UTC) #1
ckulla
LGTM But I still think there are additional corner cases to both setmessage and getmessage ...
14 years, 1 month ago (2010-02-03 01:37:23 UTC) #2
lg_imageworks.com
14 years, 1 month ago (2010-02-03 01:44:23 UTC) #3
Which other cases do you think are broken?

I can try to simplify, but it's a fairly complicated set of moving parts.  I'll
see what I can do.

	-- lg


On Feb 2, 2010, at 5:37 PM, <ckulla@gmail.com> <ckulla@gmail.com> wrote:

> LGTM
> 
> But I still think there are additional corner cases to both setmessage
> and getmessage that might be broken.
> 
> Overall the logic in these ops is quite hard to follow ... is there
> anything we could do to simplify it?
> 
> http://codereview.appspot.com/199069/show
> 

--
Larry Gritz
lg@imageworks.com




Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b