Hello GRX freaks,
I think that GRX is good library and very usefull.
Bug in GrBuildPixmapFromBits() of GRX2.2 and 2.3 found ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This problem is a memory leak. In GrBuildPixmapFromBits(), a temporaly context area is allocated from the heap-area by GrCreateContext(). But,the temporaly area isn't deallocated after this and the area is missing by the application program, because the "cwork" variable is a Auto-variable, and is destroyed at the end of this function. When you have a large memory or call this function at a few times by a your program running, this bug dose not cause the abnomal excution of the program.
The GrBuildPixmapFromBits() function is found in makepat.c Fixing the bug is easy, the following source in src/pattern/makepat.c: (I added the ~~~~~~-line part in the follow.)
GrPattern *GrBuildPixmapFromBits(char *bits,int w,int h,GrColor fgc,GrColor bgc) { GrContext csave,cwork; GrPixmap *result; unsigned char *src; int wdt,wdt2,fullw; int hgt,mask,byte;
if((fullw = _GrBestPixmapWidth(w,h)) <= 0) return(NULL); result = (GrPixmap *)malloc(sizeof(GrPixmap)); if(result == NULL) return(NULL);
if (!GrCreateContext(fullw,h,NULL,&cwork)) { // "NULL" means that internal area is secured free(result); return NULL; } csave = *CURC; *CURC = cwork; fgc &= C_COLOR; bgc &= C_COLOR; for(hgt = 0; hgt < h; hgt++) { for(wdt2 = fullw; (wdt2 -= w) >= 0; ) { src = (unsigned char *)bits; mask = byte = 0; for(wdt = w; --wdt >= 0; ) { if((mask >>= 1) == 0) { mask = 0x80; byte = *src++; } (*CURC->gc_driver->drawpixel)(wdt2+wdt,hgt,((byte & mask) ? fgc : bgc)); } } bits += (w + 7) >> 3; } *CURC = csave; result->pxp_source = cwork.gc_frame; result->pxp_source.gf_memflags = (GCM_MYCONTEXT | GCM_MYMEMORY); result->pxp_ispixmap = TRUE; result->pxp_width = fullw; result->pxp_height = h; result->pxp_oper = 0;
GrDestroyContex(&cwork); // 990923 added by A.Watanabe. // ~~~~~~~~~~~~~~~~~~~~~~~*----The above line added.
return((GrPattern *)result); }
------------- * ------------ Do you think that my pointing out a bug is correct?
with greetings, Akira Watanabe
_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/_/ Akira Watanabe A Electronics Engineer in a motor company
On Sun, 10 Oct 1999, Uncle(Akira Watanabe) wrote:
Hello GRX freaks,
I think that GRX is good library and very usefull.
Thanks.
Bug in GrBuildPixmapFromBits() of GRX2.2 and 2.3 found
This problem is a memory leak.
Well, it looks like the code is alright, see below.
In GrBuildPixmapFromBits(), a temporaly context area is allocated from the heap-area by GrCreateContext().
GrCreateContext() maps to GrCreateFrameContext() in setup/context.c:
GrContext *GrCreateFrameContext(GrFrameMode md,int w,int h,char far *memory[4],GrContext *where) { [...] if(!where) { where = malloc(sizeof(GrContext)); if(!where) return(NULL); flags = MYCONTEXT; } sttzero(where); if(!memory) { for(ii = 0; ii < fd->num_planes; ii++) { mymem[ii] = farmalloc((size_t)psize); if(!mymem[ii]) { while(--ii >= 0) farfree(mymem[ii]); if(flags) free(where); return(NULL); } } while(ii < 4) mymem[ii++] = NULL; memory = mymem; flags |= MYFRAME; }
So, memory will be allocated from heap if where==NULL and from far heap if memory==NULL
GrPattern *GrBuildPixmapFromBits(char *bits,int w,int h,GrColor fgc,GrColor bgc) { GrContext csave,cwork; GrPixmap *result; unsigned char *src; int wdt,wdt2,fullw; int hgt,mask,byte;
if((fullw = _GrBestPixmapWidth(w,h)) <= 0) return(NULL); result = (GrPixmap *)malloc(sizeof(GrPixmap)); if(result == NULL) return(NULL);
if (!GrCreateContext(fullw,h,NULL,&cwork)) { // "NULL" means that internal
This means that only frame memory will be allocated since where == &cwork != NULL
few lines later:
result->pxp_source = cwork.gc_frame; result->pxp_source.gf_memflags = (GCM_MYCONTEXT | GCM_MYMEMORY); result->pxp_ispixmap = TRUE; result->pxp_width = fullw; result->pxp_height = h; result->pxp_oper = 0; return((GrPattern *)result);
The memory in the gc_frame section of the cwork context is moved to the new created GrPattern and flags are set up so GrDestoryPattern will release the memory: (GCM_MYCONTEXT | GCM_MYMEMORY)
Do you think that my pointing out a bug is correct?
Sorry to say, but it looks like thereŽs no bug here. The memory leak occurs in the user programm if GrDestroyPattern() isnŽt called.
Clear problem of missing documentation :(
Hartmut
Thank you for your reply to me rapidly.
Sorry to say, but it looks like there$B%((Bs no bug here. The memory leak occurs in the user programm if GrDestroyPattern() isn$B%((Bt called.
It seems that my opinion isn't understanded.My application program always uses GrDestroyPattern() after GrBuildPixmapBits() called.
In GrBuildPixmapFromBits(), there are two times creating of contexts; "cwork" and "result". a)"cwork" : working area for the function. User applications can not have access to "cwork" and, It isn't necessary for users to know to the "cwork" existing. So, "cwork" and the area concerned with it must be auto-destroyed at end of this function. Auto-variable"cwork" will be automatically destroyed, but "cwork->gp_pxp_source.gf_baseaddr==memory==mymem" from far heap will not be destroyed automatically.
struct "cwork" ; from stack== auto-variable *--memory ; from far heap
b)"result" : the result of calculating in this function. A pointer "result" is bringed to user applications as a return-value and the responsibility of the destroying of "result" moves to user applications too.
About (a) : GRX library must free the area.<-------This part contain the ploblem! About (b) : User application program must free it.
GrContext *GrCreateFrameContext(GrFrameMode md,int w,int h,char far *memory[4],GrContext *where) { [...] if(!where) { where = malloc(sizeof(GrContext)); if(!where) return(NULL); flags = MYCONTEXT; } sttzero(where); if(!memory) { for(ii = 0; ii < fd->num_planes; ii++) { mymem[ii] = farmalloc((size_t)psize); if(!mymem[ii]) { while(--ii >= 0) farfree(mymem[ii]); if(flags) free(where); return(NULL); } } while(ii < 4) mymem[ii++] = NULL; memory = mymem; flags |= MYFRAME; }
So, memory will be allocated from heap if where==NULL and from far heap if memory==NULL
I understand that the condition of memory allocating are... A)from heap if where==NULL and from far heap if memory==NULL (that you mentioned ) B) only "from far heap if memory==NULL"
GrPattern *GrBuildPixmapFromBits(char *bits,int w,int h,GrColor
fgc,GrColor
bgc) { GrContext csave,cwork; GrPixmap *result; unsigned char *src; int wdt,wdt2,fullw; int hgt,mask,byte;
if((fullw = _GrBestPixmapWidth(w,h)) <= 0) return(NULL); result = (GrPixmap *)malloc(sizeof(GrPixmap)); if(result == NULL) return(NULL);
if (!GrCreateContext(fullw,h,NULL,&cwork)) { // "NULL" means that
internal
This means that only frame memory will be allocated since where == &cwork != NULL
So, this meets the above condition (B), then "memory" will be allocate from far heap. By reason of result!=&cwork, if I am willing to use GrDestroyPattern(), the apprication program can not free the local memory as "mymem"!
I don't think that this problem is my poor reading of GRX documentation. Do you think so or not?
Akira Watanabe
On Mon, 11 Oct 1999, Uncle(Akira Watanabe) wrote:
In GrBuildPixmapFromBits(), there are two times creating of contexts; "cwork" and "result".
No. result isnŽt created. Memory for result is allocated but no frame memory is allocated:
result = (GrPixmap *)malloc(sizeof(GrPixmap)); if(result == NULL) return(NULL);
Using the auto variable cwork (allocated and destroyed automaticly on the stack) the functions creates a context where to build the Pixmap:
if (!GrCreateContext(fullw,h,NULL,&cwork)) {
Note, only the frame memory will be allocated, no memory for the context info structure since &cwork != NULL
The cwork context is now used to set up the pixmap by drawing to it.
Now the new pixmap is build:
result->pxp_source = cwork.gc_frame; result->pxp_source.gf_memflags = (GCM_MYCONTEXT | GCM_MYMEMORY);
The frame memory is passed from the cwork context to the result pixmap. Setting gc_memflags to (GCM_MYCONTEXT | GCM_MYMEMORY) marks both, the pixmap info structure (GCM_MYCONTEXT) and the frame memory (GCM_MYMEMORY) to be free()Žd by GrDestoryContext(). The frame memory is now part of the pixmap and will be returned to the user. Calling GrDestroyContext on cwork would free the frame memory.
It may look to you as if everything is alright afterwards. It really isnŽt. You introduced a reference to unallocaterd memory by passing a pointer to a free()Žd area back to the user program. Everthing works fine until the same memory block is reused. This will destroy the pattern. Calling GrDestroyPatterm on the new pixmap will lead into a second free() on this area. This results in undefined behaviour and the system may do everything it want (as someone on the GCC mailing list noted: It may even start world war III if the required hardware options are installed :(
a)"cwork" : working area for the function. User applications can not have access to "cwork" and, It isn't necessary for users to know to the "cwork" existing. So, "cwork" and the area concerned with it must be auto-destroyed at end of this function. Auto-variable"cwork" will be automatically destroyed, but "cwork->gp_pxp_source.gf_baseaddr==memory==mymem" from far heap will not be destroyed automatically.
No, as said above: The frame memory allocated from far heap is part of the pixmap.
So, this meets the above condition (B), then "memory" will be allocate from far heap. By reason of result!=&cwork, if I am willing to use GrDestroyPattern(), the apprication program can not free the local memory as "mymem"!
Please note this assignment
result->pxp_source = cwork.gc_frame;
The memory reference is returned
result -> pxp_source -> frame memory
I don't think that this problem is my poor reading of GRX documentation.
No, youŽre right. The documentation is poor.
Hope I could made it somewhat clearer ...
Hartmut
-
Hartmut Schirmer -
Uncle(Akira Watanabe)