[ quesa-Bugs-1032785 ] DrawContext mask image initialisation

[SourceForge.net <no...@so...>]

Bugs item #1032785, was opened at 2004-09-23 02:44
Message generated for change (Comment added) made by bork3d
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=442052&aid=1032785&group_id=45158

Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: bork3d (bork3d)
Assigned to: Nobody/Anonymous (nobody)
Summary: DrawContext mask image initialisation

Initial Comment:
A "malloc debug: double free" occurs in 
Q3Bitmap_Empty. 
The pointer to be freed is garbage due to an uninitialised 
field in a TQ3MacDrawContextData structure declared on 
the stack. This can be fixed where the DrawingContext is 
initialized in e3drawcontext_mac_new().

/* E3MacDrawContext_New is being called without the 
mask image field explicitly set, causing problems in 
Q3Bitmap_Empty.
 * Callers _do_ however set maskState to kQ3False, 
so respect a valid mask image if maskState is kQ3True
 */
if (instanceData-
>data.macData.theData.drawContextData.maskState == 
kQ3False)
  instanceData-
>data.macData.theData.drawContextData.mask.image = 
NULL;

Similar fixes should be applied to other platform specific 
DrawingContext initialisation routines, such as 
e3drawcontext_windc_new() etc


----------------------------------------------------------------------

>Comment By: bork3d (bork3d)
Date: 2004-09-23 17:22

Message:
Logged In: YES 
user_id=1126574

> If some client code calls Q3MacDrawContext_New without 
setting
> all the fields of the TQ3MacDrawContextData structure, that's 
their bug, not Quesa's 

The clients that do not initialise the mask bitmap image field are 
actually
internal Quesa calls. For example, Q3Viewer_New and 
Q3DrawContext_New

> 3drawcontext_mac_delete does check the maskState field 
before calling Q3Bitmap_Empty

That's just one path of many to reach Q3Bitmap_Empty and 
there are many code points where the maskState could be 
checked.

The scenario to trigger the bug is:
A DrawContext is created with Q3DrawContext_New
(on Mac this calls E3MacDrawContext_NewWithWindow, 
which allocates a TQ3DrawContextData structure on the stack, 
sets the maskState to kQ3False but does not initialise the 
mask.image field, which ensures the new DrawContext object's 
mask.image is garbage).
Later one replaces the mask with Q3DrawContext_SetMask()
which calls E3Bitmap_Replace(), disposing the garbage mask 
image. 

DrawContext_New takes a TQ3DrawContextData pointer as a 
parameter.
The internal callers of DrawContext_New (and also the sample 
code on which many apps are based) follow a common pattern 
that indicates an implied software contract in the use of 
DrawContext_New():
Clients initialise the maskState field to kQ3False to indicate the 
mask is not used. These clients do not explicitly set the mask's 
bitmap image field to NULL when maskState field is kQ3False and 
pass a stack declared structure that has random values in the 
TQ3Bitmap field of the TQ3DrawContextData.

The implied contract is that if the maskState is kQ3False then 
the mask bitmap will not be accessed. The clients assume that 
just setting maskState to kQ3False is sufficent. This however 
does not mean that Quesa will not access the bitmap, as there is 
at least one case where the bitmap is accessed without regard to 
the maskState flag: Q3DrawContext_SetMask() disposes of the 
old bitmap before installing a new one. This is a problem when 
the old bitmap image field is not a valid pointer.

I assert that the software contract that DrawContext_New will 
create a fully valid DrawContext object requires the mask's 
bitmap image field must initially be NULL if imageState is 
kQ3False. The clients do not explicitly set it, so 
DrawContext_New itself must enforce this condition.

The options to ensure DrawContext_New creates a fully valid 
object are:
1. Require clients must set mask.image to NULL.
2. Ensure the condition internally during DrawContext 
initialisation.

The bottleneck routines (e3drawcontext_PLATFORM_new) are 
the candidates for ensuring mask.image is NULL when the 
maskState is kQ3False.

And finally, what does QD3D do? It accepts TQ3DrawContextData 
with maskState kQ3False and garbage image pointer (witness all 
the sample code that uses this) and Q3DrawContext_SetMask() 
does not deallocate random memory. 
So the condition is enforced in QD3D.

Devil's advocate argument: what if I want to set a mask at 
initialisation but I also set the maskState kQ3False because I 
don’t want to use the mask immediately but will later use 
Q3DrawContext_SetMaskState() to toggle the use of the mask. 
The suggested fix will null the valid bitmap image and cause a 
leak.
The workaround is to use Q3DrawContext_SetMask() after 
creating the DrawContext with null mask.



----------------------------------------------------------------------

Comment By: James W. Walker (jwwalker)
Date: 2004-09-23 03:27

Message:
Logged In: YES 
user_id=433183

I don't understand this.  If some client code calls 
Q3MacDrawContext_New 
without setting all the fields of the TQ3MacDrawContextData structure, 
that's their bug, not Quesa's.  Furthermore, e3drawcontext_mac_delete 
does check the maskState field before calling Q3Bitmap_Empty.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=442052&aid=1032785&group_id=45158