Re: Re : Re: Patch proposal about calloc leak
Yes,
but no. wmaker crash when you type Alt+Tab. Probably the patch is good, but
something is wrong in other place.
Cheers,
kix
On Thu, 15 Nov 2012, Christophe escribió:
>
> - Rodolfo García Peñas a écrit :
> > Hi,
> >
> > don't apply this patch. It causes wmaker crash using alt+tab.
>
> Hi Rodolfo,
>
> I had a look at the code, I think I can propose a patch to fix that. Thanks
> for pointing the issue, you must be quite motivated to try Valgrind!
>
> Best regards,
> Christophe.
>
>
> >
> > Cheers,
> > kix
> >
> > On Tue, 06 Nov 2012, Rodolfo kix Garcia escribió:
> >
> > > Hi,
> > >
> > > I found (not me, valgrind ;-)) a memory leak with the calloc function. I
> > > sent a patch proposal, but I am not sure if the patch is ok. Please, if
> > > more people can check the patch...
> > >
> > > Cheers,
> > > kix
> > > --
> > > ||// //\\// Rodolfo "kix" Garcia
> > > ||\\// //\\ http://www.kix.es/
> >
> > > From 60f7982cd9b48e5e1c8399b0470e6933abfc563f Mon Sep 17 00:00:00 2001
> > > From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
> > >
> > > Date: Tue, 6 Nov 2012 22:59:13 +0100
> > > Subject: [PATCH] calloc free memory leak
> > >
> > > The memory allocated using calloc is not properly freed.
> > >
> > > This is the valgrind error:
> > >
> > > 5 errors in context 11 of 12:
> > > Syscall param writev(vector[...]) points to uninitialised byte(s)
> > > at 0x430A65B: writev (writev.c:51)
> > > by 0x44F17C5: ??? (in /usr/lib/i386-linux-gnu/libxcb.so.1.1.0)
> > > by 0x42180AF: ??? (in /usr/lib/i386-linux-gnu/libX11.so.6.3.0)
> > > Address 0x469f475 is 421 bytes inside a block of size 16,384 alloc'd
> > > at 0x4026A68: calloc (vg_replace_malloc.c:566)
> > > by 0x410EAE9: XOpenDisplay (in
> > > /usr/lib/i386-linux-gnu/libX11.so.6.3.0)
> > > by 0x3D474E40: ???
> > >
> > > The memory is allocated in this way:
> > >
> > > contrib = (CLIST *) calloc(new_width, sizeof(CLIST));
> > >
> > > And is freed in this way:
> > >
> > >for (i = 0; i < dst->height; ++i)
> > > free(contrib[i].p);
> > >
> > > But dst->height can be less than the number of elements (contrib[i].n)
> > > The correct way should be:
> > >
> > > for (i = 0; i < tmp->height; i++) {
> > > for (k = 0; k < contrib[i].n; k++)
> > > free(contrib[i].p);
> > > }
> > >
> > > After this patch, valdgrind doesn't report errors.
> > >
> > > This patch also includes the variable contrib inside the function (now is
> > > not global) and some style changes.
> > > ---
> > > wrlib/scale.c | 21 +++--
> > > 1 file changed, 11 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/wrlib/scale.c b/wrlib/scale.c
> > > index 82ed57e..6bbf3c6 100644
> > > --- a/wrlib/scale.c
> > > +++ b/wrlib/scale.c
> > > @@ -278,14 +278,13 @@ typedef struct {
> > > CONTRIB *p; /* pointer to list of contributions */
> > > } CLIST;
> > >
> > > -CLIST *contrib; /* array of contribution lists */
> > > -
> > > /* clamp the input to the specified range */
> > > #define CLAMP(v,l,h)((v)<(l) ? (l) : (v) > (h) ? (h) : v)
> > >
> > > /* return of calloc is not checked if NULL in the function below! */
> > > -RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned
> > > new_height)
> > > +RImage *RSmoothScaleImage(RImage *src, unsigned new_width, unsigned
> > > new_height)
> > > {
> > > + CLIST *contrib; /* array of contribution lists */
> > > RImage *tmp;/* intermediate image */
> > > double xscale, yscale; /* zoom scale factors */
> > > int i, j, k;/* loop variables */
> > > @@ -294,8 +293,7 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > > new_width, unsigned new_height)
> > > double width, fscale; /* filter calculation variables */
> > > double rweight, gweight, bweight;
> > > RImage *dst;
> > > - unsigned char *p;
> > > - unsigned char *sp;
> > > + unsigned char *p, *sp;
> > > int sch = src->format == RRGBAFormat ? 4 : 3;
> > >
> > > dst = RCreateImage(new_width, new_height, False);
> > > @@ -332,7 +330,6 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > > new_width, unsigned new_height)
> > > }
> > > }
> > > } else {
> > > -
> > > for (i = 0; i < new_width; ++i) {
> > > contrib[i].n = 0;
> > > contrib[i].p = (CONTRIB *) calloc((int) ceil(fwidth * 2
> > > + 1), sizeof(CONTRIB));
> > > @@ -381,9 +378,11 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > > new_width, unsigned new_height)
> > > }
> > >
> > > /* free the memory allocated for horizontal filter weights */
> > > - for (i = 0; i < tmp->width; ++i) {
> > > - free(contrib[i].p);
> > > + for (i = 0; i < tmp->width; i++) {
> > > + for (k = 0; k < contrib[i].n; k++)
> > > + free(contrib[i].p);
> > > }
> > > +
> > > free(contrib);
> > >
> > > /*
Re: [PATCH] get_wwindow_image_from_wmhints scale image
On Wed, 14 Nov 2012, Rodolfo García Peñas wrote: Then, the image can be resized using wIconValidateIconSize(). The resize should be done in get_rimage_icon_from_wm_hints(), not in the function get_wwindow_image_from_wmhints(). This is because the function get_wwindow_image_from_wmhints() is used in wIconStore() too. If we resize the image before save it to disk, then if we change the icon/dock size, then the image saved will have a different size than the curren icon size. Is better resize the image when is painted in the screen, not the image saved. Thanks a lot for fixing all this. I couldn't really follow all your changes so I'm not sure how it all works now so here are some questions/thoughts. I think you've thought about these already but asking it just in case: - This does not mean that the image is resized every time it is painted, does it? If yes then it may be faster to check the size of the cached image before using it and remove it when the icon size has been changed. - Images are also enlarged when they are too small? This may look bad so probably there could be a limit (say no more than 2x) or only scale down images if they are too big but never enlarge them. - Is it possible to change icons of applications without restarting them (the Apply button in the inspector is working) and does the ignore client supplied icon option work? Regards, BALATON Zoltan
[PATCH] WPrefs: More moving around of options and tweaks to layout
From a44cdb52b62959a11416c86103231c4b8af3b8e8 Mon Sep 17 00:00:00 2001
From: BALATON Zoltan
Date: Thu, 15 Nov 2012 02:53:50 +0100
Subject: [PATCH] WPrefs: More moving around of options and tweaks to layout
Managed to squeeze two more options from the expert page to the
corresponding pages and also reorganised some widgets to avoid large
now unused spaces in one place and clipped text in the other.
---
WPrefs.app/Configurations.c | 70 ++--
WPrefs.app/Expert.c | 30 +++-
WPrefs.app/Focus.c | 30
WPrefs.app/Icons.c | 82 ---
WPrefs.app/Preferences.c| 33 -
WPrefs.app/WindowHandling.c | 77 ++--
WPrefs.app/po/ja.po |2 +-
7 files changed, 167 insertions(+), 157 deletions(-)
diff --git a/WPrefs.app/Configurations.c b/WPrefs.app/Configurations.c
index 952fd26..e64d8c1 100644
--- a/WPrefs.app/Configurations.c
+++ b/WPrefs.app/Configurations.c
@@ -173,14 +173,14 @@ static void createPanel(Panel *p)
/*** Icon Slide Speed **/
panel->icoF = WMCreateFrame(panel->box);
- WMResizeWidget(panel->icoF, 230, 45);
+ WMResizeWidget(panel->icoF, 212, 45);
WMMoveWidget(panel->icoF, 15, 10);
WMSetFrameTitle(panel->icoF, _("Icon Slide Speed"));
/*** Shade Animation Speed **/
panel->shaF = WMCreateFrame(panel->box);
- WMResizeWidget(panel->shaF, 230, 45);
- WMMoveWidget(panel->shaF, 15, 60);
+ WMResizeWidget(panel->shaF, 212, 45);
+ WMMoveWidget(panel->shaF, 15, 65);
WMSetFrameTitle(panel->shaF, _("Shade Animation Speed"));
buf1 = wmalloc(strlen(SPEED_IMAGE) + 1);
@@ -190,9 +190,9 @@ static void createPanel(Panel *p)
panel->icoB[i] = WMCreateCustomButton(panel->icoF,
WBBStateChangeMask);
panel->shaB[i] = WMCreateCustomButton(panel->shaF,
WBBStateChangeMask);
WMResizeWidget(panel->icoB[i], 40, 24);
- WMMoveWidget(panel->icoB[i], 10 + (40 * i), 15);
+ WMMoveWidget(panel->icoB[i], 2 + (40 * i), 15);
WMResizeWidget(panel->shaB[i], 40, 24);
- WMMoveWidget(panel->shaB[i], 10 + (40 * i), 15);
+ WMMoveWidget(panel->shaB[i], 2 + (40 * i), 15);
WMSetButtonBordered(panel->icoB[i], False);
WMSetButtonImagePosition(panel->icoB[i], WIPImageOnly);
if (i > 0) {
@@ -238,8 +238,8 @@ static void createPanel(Panel *p)
/* Smoothed Scaling */
panel->smoF = WMCreateFrame(panel->box);
- WMResizeWidget(panel->smoF, 115, 110);
- WMMoveWidget(panel->smoF, 18, 115);
+ WMResizeWidget(panel->smoF, 94, 100);
+ WMMoveWidget(panel->smoF, 420, 10);
WMSetFrameTitle(panel->smoF, _("Smooth Scaling"));
WMSetBalloonTextForView(_("Smooth scaled background images,
neutralizing\n"
"the `pixelization' effect. This will slow\n"
@@ -247,7 +247,7 @@ static void createPanel(Panel *p)
panel->smoB = WMCreateButton(panel->smoF, WBTToggle);
WMResizeWidget(panel->smoB, 64, 64);
- WMMoveWidget(panel->smoB, 25, 25);
+ WMMoveWidget(panel->smoB, 15, 23);
WMSetButtonImagePosition(panel->smoB, WIPImageOnly);
path = LocateImage(SMOOTH_IMAGE);
if (path) {
@@ -279,41 +279,41 @@ static void createPanel(Panel *p)
/* Titlebar Style Size /
panel->titlF = WMCreateFrame(panel->box);
- WMResizeWidget(panel->titlF, 105, 110);
- WMMoveWidget(panel->titlF, 140, 115);
+ WMResizeWidget(panel->titlF, 212, 97);
+ WMMoveWidget(panel->titlF, 15, 120);
WMSetFrameTitle(panel->titlF, _("Titlebar Style"));
- panel->newsB = WMCreateButton(panel->titlF, WBTOnOff);
- WMResizeWidget(panel->newsB, 74, 40);
- WMMoveWidget(panel->newsB, 15, 20);
- WMSetButtonImagePosition(panel->newsB, WIPImageOnly);
- path = LocateImage(NEWS_IMAGE);
+ panel->oldsB = WMCreateButton(panel->titlF, WBTOnOff);
+ WMResizeWidget(panel->oldsB, 60, 40);
+ WMMoveWidget(panel->oldsB, 16, 32);
+ WMSetButtonImagePosition(panel->oldsB, WIPImageOnly);
+ path = LocateImage(OLDS_IMAGE);
if (path) {
icon = WMCreatePixmapFromFile(scr, path);
if (icon) {
- WMSetButtonImage(panel->newsB, icon);
+ WMSetButtonImage(panel->oldsB, icon);
WMReleasePixmap(icon);
}
wfree(path);
}
- panel->oldsB = WMCreateButton(panel->titlF, WBTOnOff);
-WMResizeWidget(panel->oldsB, 37, 40);
- WMMoveWidget(panel->oldsB, 15, 60);
- WMSetButtonImagePosition(panel->oldsB, WIPImageOnly);
- p
[PATCH] Fixed wrong count to release temporary memory
From ddc5f4d3b360b08bedeefbe03c404d3dd98abd38 Mon Sep 17 00:00:00 2001
From: Christophe CURIS
Date: Thu, 15 Nov 2012 01:30:33 +0100
Subject: [PATCH] Fixed wrong count to release temporary memory
As found by Rodolfo, it looks like there could be memory leak in the
function 'RSmoothScaleImage' because it reserveda given number of
memory blocs but used another count to free them after use.
This patch uses the same count for release as it seems this variable
is not modified in between. Took the opportunity as Rodolfo proposed
to convert a global variable to a local variable - this global
definition seems incorrect.
---
wrlib/scale.c |5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/wrlib/scale.c b/wrlib/scale.c
index 82ed57e..c5b5caf 100644
--- a/wrlib/scale.c
+++ b/wrlib/scale.c
@@ -278,14 +278,13 @@ typedef struct {
CONTRIB *p; /* pointer to list of contributions */
} CLIST;
-CLIST *contrib; /* array of contribution lists */
-
/* clamp the input to the specified range */
#define CLAMP(v,l,h)((v)<(l) ? (l) : (v) > (h) ? (h) : v)
/* return of calloc is not checked if NULL in the function below! */
RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height)
{
+ CLIST *contrib; /* array of contribution lists */
RImage *tmp; /* intermediate image */
double xscale, yscale; /* zoom scale factors */
int i, j, k; /* loop variables */
@@ -381,7 +380,7 @@ RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned new_height)
}
/* free the memory allocated for horizontal filter weights */
- for (i = 0; i < tmp->width; ++i) {
+ for (i = 0; i < new_width; ++i) {
free(contrib[i].p);
}
free(contrib);
--
1.7.10.4
Re : Re: Patch proposal about calloc leak
- Rodolfo García Peñas a écrit :
> Hi,
>
> don't apply this patch. It causes wmaker crash using alt+tab.
Hi Rodolfo,
I had a look at the code, I think I can propose a patch to fix that. Thanks for
pointing the issue, you must be quite motivated to try Valgrind!
Best regards,
Christophe.
>
> Cheers,
> kix
>
> On Tue, 06 Nov 2012, Rodolfo kix Garcia escribió:
>
> > Hi,
> >
> > I found (not me, valgrind ;-)) a memory leak with the calloc function. I
> > sent a patch proposal, but I am not sure if the patch is ok. Please, if
> > more people can check the patch...
> >
> > Cheers,
> > kix
> > --
> > ||// //\\// Rodolfo "kix" Garcia
> > ||\\// //\\ http://www.kix.es/
>
> > From 60f7982cd9b48e5e1c8399b0470e6933abfc563f Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
> > Date: Tue, 6 Nov 2012 22:59:13 +0100
> > Subject: [PATCH] calloc free memory leak
> >
> > The memory allocated using calloc is not properly freed.
> >
> > This is the valgrind error:
> >
> > 5 errors in context 11 of 12:
> > Syscall param writev(vector[...]) points to uninitialised byte(s)
> > at 0x430A65B: writev (writev.c:51)
> > by 0x44F17C5: ??? (in /usr/lib/i386-linux-gnu/libxcb.so.1.1.0)
> > by 0x42180AF: ??? (in /usr/lib/i386-linux-gnu/libX11.so.6.3.0)
> > Address 0x469f475 is 421 bytes inside a block of size 16,384 alloc'd
> > at 0x4026A68: calloc (vg_replace_malloc.c:566)
> > by 0x410EAE9: XOpenDisplay (in /usr/lib/i386-linux-gnu/libX11.so.6.3.0)
> > by 0x3D474E40: ???
> >
> > The memory is allocated in this way:
> >
> > contrib = (CLIST *) calloc(new_width, sizeof(CLIST));
> >
> > And is freed in this way:
> >
> >for (i = 0; i < dst->height; ++i)
> > free(contrib[i].p);
> >
> > But dst->height can be less than the number of elements (contrib[i].n)
> > The correct way should be:
> >
> > for (i = 0; i < tmp->height; i++) {
> > for (k = 0; k < contrib[i].n; k++)
> > free(contrib[i].p);
> > }
> >
> > After this patch, valdgrind doesn't report errors.
> >
> > This patch also includes the variable contrib inside the function (now is
> > not global) and some style changes.
> > ---
> > wrlib/scale.c | 21 +++--
> > 1 file changed, 11 insertions(+), 10 deletions(-)
> >
> > diff --git a/wrlib/scale.c b/wrlib/scale.c
> > index 82ed57e..6bbf3c6 100644
> > --- a/wrlib/scale.c
> > +++ b/wrlib/scale.c
> > @@ -278,14 +278,13 @@ typedef struct {
> > CONTRIB *p; /* pointer to list of contributions */
> > } CLIST;
> >
> > -CLIST *contrib;/* array of contribution lists */
> > -
> > /* clamp the input to the specified range */
> > #define CLAMP(v,l,h)((v)<(l) ? (l) : (v) > (h) ? (h) : v)
> >
> > /* return of calloc is not checked if NULL in the function below! */
> > -RImage *RSmoothScaleImage(RImage * src, unsigned new_width, unsigned
> > new_height)
> > +RImage *RSmoothScaleImage(RImage *src, unsigned new_width, unsigned
> > new_height)
> > {
> > + CLIST *contrib; /* array of contribution lists */
> > RImage *tmp;/* intermediate image */
> > double xscale, yscale; /* zoom scale factors */
> > int i, j, k;/* loop variables */
> > @@ -294,8 +293,7 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > new_width, unsigned new_height)
> > double width, fscale; /* filter calculation variables */
> > double rweight, gweight, bweight;
> > RImage *dst;
> > - unsigned char *p;
> > - unsigned char *sp;
> > + unsigned char *p, *sp;
> > int sch = src->format == RRGBAFormat ? 4 : 3;
> >
> > dst = RCreateImage(new_width, new_height, False);
> > @@ -332,7 +330,6 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > new_width, unsigned new_height)
> > }
> > }
> > } else {
> > -
> > for (i = 0; i < new_width; ++i) {
> > contrib[i].n = 0;
> > contrib[i].p = (CONTRIB *) calloc((int) ceil(fwidth * 2
> > + 1), sizeof(CONTRIB));
> > @@ -381,9 +378,11 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > new_width, unsigned new_height)
> > }
> >
> > /* free the memory allocated for horizontal filter weights */
> > - for (i = 0; i < tmp->width; ++i) {
> > - free(contrib[i].p);
> > + for (i = 0; i < tmp->width; i++) {
> > + for (k = 0; k < contrib[i].n; k++)
> > + free(contrib[i].p);
> > }
> > +
> > free(contrib);
> >
> > /* pre-calculate filter contributions for a column */
> > @@ -475,9 +474,11 @@ RImage *RSmoothScaleImage(RImage * src, unsigned
> > new_width, unsigned new_height)
> > free(sp);
> >
> > /* free the memory allocated for vertical filter weights */
> > - for (i = 0; i < dst->height; ++i) {
> > - free(contrib[i].p);
> > + for (i = 0; i < tmp->height; i++) {
> > + for (k
Re: [PATCH] get_default_image resize image
On Thu, 15 Nov 2012, Rodolfo kix Garcia escribió:
> On Wed, 14 Nov 2012, Carlos R. Mafra escribió:
>
> > Thanks a lot for your latest series! Now the appicon from gtk-demo
> > is nicely resized!
> >
> > Gracias Rodolfo!
>
> Thanks.
>
> I am near to finish the winspector problem with icons. I think I will have
> the patches tomorrow.
I got it. I will rebase all here tomorrow and I will send it. The winspector.c
(applySettings()) is something like:
char *file = WMGetTextFieldText(panel->fileText);
if (file[0] == 0) {
wfree(file);
file = NULL;
}
if (!WFLAGP(wwin, always_user_icon)) {
/* Change icon image if the app is minimized */
if (wwin->icon)
wIconChangeImageFile(wwin->icon, file);
/* Change App Icon image */
if (wapp->app_icon)
wIconChangeImageFile(wapp->app_icon->icon,
file);
} else {
/* Change App Icon image */
if (wapp->app_icon)
wIconUpdate(wapp->app_icon->icon,
get_rimage_icon_from_wm_hints(wapp->app_icon->icon));
/* Change icon image if the app is minimized */
if (wwin->icon)
wIconUpdate(wwin->icon,
get_rimage_icon_from_wm_hints(wwin->icon));
}
if (file)
wfree(file);
Now wIconUpdate is more configurable, and the code is clean. But I made a lot
of changes...
Regards, good nigth,
kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: [PATCH] get_default_image resize image
On Wed, 14 Nov 2012, Carlos R. Mafra escribió: > Thanks a lot for your latest series! Now the appicon from gtk-demo > is nicely resized! > > Gracias Rodolfo! Thanks. I am near to finish the winspector problem with icons. I think I will have the patches tomorrow. -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/ -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.3-189-g2eb1107
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project wmaker-crm.git.
The branch, next has been updated
via 2eb1107e20f889ef88f0055b8544b950dd5024a5 (commit)
via d8b92c979e16c527fbb1c1f0631eeca9aa8ad042 (commit)
via 78ff715d39432e44fd57ce91c940ceeaa6b76a23 (commit)
via 5956d71d77fcb76273c6eccaab2347276c04fbcd (commit)
via ead0fb2e4bf14eadf22b23987fd41bc60c6635b1 (commit)
via 7746fe7c5a46a58eb5a2e5393487de8213bb0a42 (commit)
from 5dcd31acbe94959656e4d971c0f4b462a0bfb7c7 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -
http://repo.or.cz/w/wmaker-crm.git/commit/2eb1107e20f889ef88f0055b8544b950dd5024a5
commit 2eb1107e20f889ef88f0055b8544b950dd5024a5
Author: Rodolfo GarcÃa Peñas (kix)
Date: Wed Nov 14 22:11:49 2012 +0100
wIconUpdate removed scr variable
The variable scr is not used, so MUST be removed.
This variable can cause a segfault because icon could not exist.
diff --git a/src/icon.c b/src/icon.c
index 1ed90bd..6a0e817 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -601,11 +601,8 @@ static void unset_icon_image(WIcon *icon)
void wIconUpdate(WIcon *icon)
{
- WScreen *scr = icon->core->screen_ptr;
WWindow *wwin = icon->owner;
- assert(scr->icon_tile != NULL);
-
if (wwin && WFLAGP(wwin, always_user_icon)) {
/* Forced use user_icon */
get_rimage_icon_from_user_icon(icon);
http://repo.or.cz/w/wmaker-crm.git/commit/d8b92c979e16c527fbb1c1f0631eeca9aa8ad042
commit d8b92c979e16c527fbb1c1f0631eeca9aa8ad042
Author: Rodolfo GarcÃa Peñas (kix)
Date: Wed Nov 14 19:00:22 2012 +0100
get_default_image resize image
The function get_default_image, used to read the default image, now
resizes it to the desired size.
diff --git a/src/wdefaults.c b/src/wdefaults.c
index a613a31..0e7d6cd 100644
--- a/src/wdefaults.c
+++ b/src/wdefaults.c
@@ -466,6 +466,10 @@ RImage *get_default_image(WScreen *scr)
if (!image)
wwarning(_("could not find default icon "%s""), path);
+ /* Resize the icon to the wPreferences.icon_size size
+* usually this function will return early, because size is right */
+ image = wIconValidateIconSize(image, wPreferences.icon_size);
+
return image;
}
http://repo.or.cz/w/wmaker-crm.git/commit/78ff715d39432e44fd57ce91c940ceeaa6b76a23
commit 78ff715d39432e44fd57ce91c940ceeaa6b76a23
Author: Rodolfo GarcÃa Peñas (kix)
Date: Wed Nov 14 19:26:12 2012 +0100
wIconValidateIconSize checks the width and height
The function wIconValidateIconSize checks if the width size and height size
are
less than the preference size (and left space for the border). If the width
size or height size is greater than the preference, then checks what is the
bigger size of them. Then resize it using the bigger value and holding the
aspect ratio.
Before this patch, wIconValidateIconSize didn't check if height was bigger
than width and always suppose that width was greater than height.
diff --git a/src/icon.c b/src/icon.c
index 166c827..1ed90bd 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -359,10 +359,14 @@ RImage *wIconValidateIconSize(RImage *icon, int max_size)
return NULL;
/* We should hold "ICON_BORDER" (~2) pixels to include the icon border
*/
- if ((icon->width - max_size) > -ICON_BORDER ||
- (icon->height - max_size) > -ICON_BORDER) {
- nimage = RScaleImage(icon, max_size - ICON_BORDER,
-(icon->height * (max_size - ICON_BORDER) /
icon->width));
+ if (((max_size + ICON_BORDER) < icon->width) ||
+ ((max_size + ICON_BORDER) < icon->height)) {
+ if (icon->width > icon->height)
+ nimage = RScaleImage(icon, max_size - ICON_BORDER,
+(icon->height * (max_size -
ICON_BORDER) / icon->width));
+ else
+ nimage = RScaleImage(icon, (icon->width * (max_size -
ICON_BORDER) / icon->height),
+max_size - ICON_BORDER);
RReleaseImage(icon);
icon = nimage;
}
http://repo.or.cz/w/wmaker-crm.git/commit/5956d71d77fcb76273c6eccaab2347276c04fbcd
commit 5956d71d77fcb76273c6eccaab2347276c04fbcd
Author: Rodolfo GarcÃa Peñas (kix)
Date: Wed Nov 14 18:36:25 2012 +0100
get_wwindow_image_from_wmhints scale image
The function get_wwindow_image_from_wmhints returns a image from WM Hints,
but the image could be larger than the desired.
Then, the image can be resized using w
Re: [PATCH] get_default_image resize image
Thanks a lot for your latest series! Now the appicon from gtk-demo is nicely resized! Gracias Rodolfo! -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[no subject]
>From ac58e775bf4807d73d44625c6d07b7bfcec1a757 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 22:11:49 +0100
Subject: [PATCH] wIconUpdate removed scr variable
The variable scr is not used, so MUST be removed.
This variable can cause a segfault because icon could not exist.
---
src/icon.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/src/icon.c b/src/icon.c
index ba059ad..c56122f 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -593,11 +593,8 @@ static void unset_icon_image(WIcon *icon)
void wIconUpdate(WIcon *icon)
{
- WScreen *scr = icon->core->screen_ptr;
WWindow *wwin = icon->owner;
- assert(scr->icon_tile != NULL);
-
if (wwin && WFLAGP(wwin, always_user_icon)) {
/* Forced use user_icon */
get_rimage_icon_from_user_icon(icon);
--
1.7.10.4
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
>From ac58e775bf4807d73d44625c6d07b7bfcec1a757 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 22:11:49 +0100
Subject: [PATCH] wIconUpdate removed scr variable
The variable scr is not used, so MUST be removed.
This variable can cause a segfault because icon could not exist.
---
src/icon.c |3 ---
1 file changed, 3 deletions(-)
diff --git a/src/icon.c b/src/icon.c
index ba059ad..c56122f 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -593,11 +593,8 @@ static void unset_icon_image(WIcon *icon)
void wIconUpdate(WIcon *icon)
{
- WScreen *scr = icon->core->screen_ptr;
WWindow *wwin = icon->owner;
- assert(scr->icon_tile != NULL);
-
if (wwin && WFLAGP(wwin, always_user_icon)) {
/* Forced use user_icon */
get_rimage_icon_from_user_icon(icon);
--
1.7.10.4
[PATCH] get_default_image resize image
>From c2a8bcb9447de353a71b443b7e03b03775acabbc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 19:00:22 +0100
Subject: [PATCH] get_default_image resize image
The function get_default_image, used to read the default image, now
resize it to the desired size.
---
src/wdefaults.c |4
1 file changed, 4 insertions(+)
diff --git a/src/wdefaults.c b/src/wdefaults.c
index a613a31..0e7d6cd 100644
--- a/src/wdefaults.c
+++ b/src/wdefaults.c
@@ -466,6 +466,10 @@ RImage *get_default_image(WScreen *scr)
if (!image)
wwarning(_("could not find default icon \"%s\""), path);
+ /* Resize the icon to the wPreferences.icon_size size
+* usually this function will return early, because size is right */
+ image = wIconValidateIconSize(image, wPreferences.icon_size);
+
return image;
}
--
1.7.10.4
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
>From c2a8bcb9447de353a71b443b7e03b03775acabbc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 19:00:22 +0100
Subject: [PATCH] get_default_image resize image
The function get_default_image, used to read the default image, now
resize it to the desired size.
---
src/wdefaults.c |4
1 file changed, 4 insertions(+)
diff --git a/src/wdefaults.c b/src/wdefaults.c
index a613a31..0e7d6cd 100644
--- a/src/wdefaults.c
+++ b/src/wdefaults.c
@@ -466,6 +466,10 @@ RImage *get_default_image(WScreen *scr)
if (!image)
wwarning(_("could not find default icon \"%s\""), path);
+ /* Resize the icon to the wPreferences.icon_size size
+ * usually this function will return early, because size is right */
+ image = wIconValidateIconSize(image, wPreferences.icon_size);
+
return image;
}
--
1.7.10.4
[PATCH] wIconValidateIconSize checks the width and height
>From 8ff93eb33a3737f28516301cede3d16930c31a31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 19:26:12 +0100
Subject: [PATCH] wIconValidateIconSize checks the width and height
The function wIconValidateIconSize checks if the width size and height size are
less than the preference size (and left space for the border). If the width
size or height size is greater than the preference, then checks what is the
bigger size of them. Then resize it using the bigger value and holding the
aspect ratio.
Before this patch, wIconValidateIconSize didn't check if height was bigger
than width and always suppose that width was greater than height.
---
src/icon.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/icon.c b/src/icon.c
index 9b40e76..2a5fcaa 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -359,10 +359,14 @@ RImage *wIconValidateIconSize(RImage *icon, int max_size)
return NULL;
/* We should hold "ICON_BORDER" (~2) pixels to include the icon border
*/
- if ((icon->width - max_size) > -ICON_BORDER ||
- (icon->height - max_size) > -ICON_BORDER) {
- nimage = RScaleImage(icon, max_size - ICON_BORDER,
-(icon->height * (max_size - ICON_BORDER) /
icon->width));
+ if (((max_size + ICON_BORDER) < icon->width) ||
+ ((max_size + ICON_BORDER) < icon->height)) {
+ if (icon->width > icon->height)
+ nimage = RScaleImage(icon, max_size - ICON_BORDER,
+(icon->height * (max_size -
ICON_BORDER) / icon->width));
+ else
+ nimage = RScaleImage(icon, (icon->width * (max_size -
ICON_BORDER) / icon->height),
+max_size - ICON_BORDER);
RReleaseImage(icon);
icon = nimage;
}
--
1.7.10.4
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
>From 8ff93eb33a3737f28516301cede3d16930c31a31 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 19:26:12 +0100
Subject: [PATCH] wIconValidateIconSize checks the width and height
The function wIconValidateIconSize checks if the width size and height size are
less than the preference size (and left space for the border). If the width
size or height size is greater than the preference, then checks what is the
bigger size of them. Then resize it using the bigger value and holding the
aspect ratio.
Before this patch, wIconValidateIconSize didn't check if height was bigger
than width and always suppose that width was greater than height.
---
src/icon.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/icon.c b/src/icon.c
index 9b40e76..2a5fcaa 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -359,10 +359,14 @@ RImage *wIconValidateIconSize(RImage *icon, int max_size)
return NULL;
/* We should hold "ICON_BORDER" (~2) pixels to include the icon border */
- if ((icon->width - max_size) > -ICON_BORDER ||
- (icon->height - max_size) > -ICON_BORDER) {
- nimage = RScaleImage(icon, max_size - ICON_BORDER,
- (icon->height * (max_size - ICON_BORDER) / icon->width));
+ if (((max_size + ICON_BORDER) < icon->width) ||
+ ((max_size + ICON_BORDER) < icon->height)) {
+ if (icon->width > icon->height)
+ nimage = RScaleImage(icon, max_size - ICON_BORDER,
+ (icon->height * (max_size - ICON_BORDER) / icon->width));
+ else
+ nimage = RScaleImage(icon, (icon->width * (max_size - ICON_BORDER) / icon->height),
+ max_size - ICON_BORDER);
RReleaseImage(icon);
icon = nimage;
}
--
1.7.10.4
[PATCH] get_wwindow_image_from_wmhints scale image
>From 8408fc131376e8bed574b3b92c0544cad4241cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?= Date: Wed, 14 Nov 2012 18:36:25 +0100 Subject: [PATCH] get_wwindow_image_from_wmhints scale image The function get_wwindow_image_from_wmhints returns a image from WM Hints, but the image could be larger than the desired. Then, the image can be resized using wIconValidateIconSize(). The resize should be done in get_rimage_icon_from_wm_hints(), not in the function get_wwindow_image_from_wmhints(). This is because the function get_wwindow_image_from_wmhints() is used in wIconStore() too. If we resize the image before save it to disk, then if we change the icon/dock size, then the image saved will have a different size than the curren icon size. Is better resize the image when is painted in the screen, not the image saved. --- src/icon.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/icon.c b/src/icon.c index ba059ad..aec3c6a 100644 --- a/src/icon.c +++ b/src/icon.c @@ -755,6 +755,9 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon) if (!image) return 1; + /* Resize the icon to the wPreferences.icon_size size */ + image = wIconValidateIconSize(image, wPreferences.icon_size); + /* FIXME: If unset_icon_image, pointer double free then crash unset_icon_image(icon); */ icon->file_image = image; -- 1.7.10.4 -- ||// //\\// Rodolfo "kix" Garcia ||\\// //\\ http://www.kix.es/ >From 8408fc131376e8bed574b3b92c0544cad4241cb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?= Date: Wed, 14 Nov 2012 18:36:25 +0100 Subject: [PATCH] get_wwindow_image_from_wmhints scale image The function get_wwindow_image_from_wmhints returns a image from WM Hints, but the image could be larger than the desired. Then, the image can be resized using wIconValidateIconSize(). The resize should be done in get_rimage_icon_from_wm_hints(), not in the function get_wwindow_image_from_wmhints(). This is because the function get_wwindow_image_from_wmhints() is used in wIconStore() too. If we resize the image before save it to disk, then if we change the icon/dock size, then the image saved will have a different size than the curren icon size. Is better resize the image when is painted in the screen, not the image saved. --- src/icon.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/icon.c b/src/icon.c index ba059ad..aec3c6a 100644 --- a/src/icon.c +++ b/src/icon.c @@ -755,6 +755,9 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon) if (!image) return 1; + /* Resize the icon to the wPreferences.icon_size size */ + image = wIconValidateIconSize(image, wPreferences.icon_size); + /* FIXME: If unset_icon_image, pointer double free then crash unset_icon_image(icon); */ icon->file_image = image; -- 1.7.10.4
Re: [PATCH] Added reset of pointer after memory free to avoid double-free crash
You can add this to your patch
Cheers,
kix
On Wed, 14 Nov 2012, Christophe escribió:
> From 0bdc1cabc08f612b4a29f03e50051a15f2acf71c Mon Sep 17 00:00:00 2001
> From: Christophe CURIS
> Date: Wed, 14 Nov 2012 21:23:32 +0100
> Subject: [PATCH] Added reset of pointer after memory free to avoid
> double-free crash
>
> As reported by Rodolfo, there are some cases when working with icons
> where a crash can occur, which is related to trying to re-release
> some memory that have been already freed previously.
>
> This patch adds a reset-to-NULL of the corresponding pointers so that
> on next usage wmaker will know there's no more memory associated with
> these pointers.
> ---
> src/icon.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/src/icon.c b/src/icon.c
> index ba059ad..04767d8 100644
> --- a/src/icon.c
> +++ b/src/icon.c
> @@ -584,11 +584,15 @@ void wIconSelect(WIcon * icon)
>
> static void unset_icon_image(WIcon *icon)
> {
> - if (icon->file)
> + if (icon->file) {
> wfree(icon->file);
> + icon->file = NULL;
> + }
>
> - if (icon->file_image)
> + if (icon->file_image) {
> RReleaseImage(icon->file_image);
> + icon->file_image = NULL;
> + }
> }
>
> void wIconUpdate(WIcon *icon)
> @@ -755,8 +759,7 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
> if (!image)
> return 1;
>
> - /* FIXME: If unset_icon_image, pointer double free then crash
> - unset_icon_image(icon); */
> + unset_icon_image(icon);
> icon->file_image = image;
>
> return 0;
> --
> 1.7.10.4
>
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
>From db2d892eb592d38cc9740e79ef765981f492901a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?"Rodolfo=20Garc=C3=ADa=20Pe=C3=B1as=20(kix)"?=
Date: Wed, 14 Nov 2012 20:06:14 +0100
Subject: [PATCH] Remove dup set icon file to NULL
Remove the icon->file = NULL because was set in unset_icon_image().
---
src/icon.c | 3 ---
1 file changed, 0 insertions(+), 3 deletions(-)
diff --git a/src/icon.c b/src/icon.c
index aec3c6a..9b40e76 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -652,7 +655,6 @@ static void get_rimage_icon_from_x11(WIcon *icon)
unset_icon_image(icon);
/* Set the new icon image */
- icon->file = NULL;
icon->file_image = RRetainImage(icon->owner->net_icon_image);
}
@@ -676,7 +678,6 @@ static void get_rimage_icon_from_default_icon(WIcon *icon)
unset_icon_image(icon);
/* Set the new icon image */
- icon->file = NULL;
icon->file_image = RRetainImage(scr->def_icon_rimage);
}
@@ -692,7 +693,6 @@ static void get_rimage_icon_from_icon_win(WIcon *icon)
unset_icon_image(icon);
/* Set the new info */
- icon->file = NULL;
icon->file_image = image;
}
--
1.7.10.4
Re: Re : Re: Help with wfree
On Wed, 14 Nov 2012, Christophe escribió:
>
> - Carlos R. Mafra a écrit :
> > On Wed, 14 Nov 2012 at 20:18:11 +0100, Rodolfo García Peñas wrote:
> > > On Wed, 14 Nov 2012, Carlos R. Mafra escribió:
> > >
> > > > On Wed, 14 Nov 2012 at 19:34:42 +0100, Rodolfo García Peñas wrote:
> > > > > Ok,
> > > > >
> > > > > seems to be ok now :-)
> > > > >
> > > > > --- a/src/icon.c
> > > > > +++ b/src/icon.c
> > > > > @@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon
> > > > > *icon)
> > > > > /* Resize the icon to the wPreferences.icon_size size */
> > > > > image = wIconValidateIconSize(image, wPreferences.icon_size);
> > > > >
> > > > > - /* FIXME: If unset_icon_image, pointer double free then crash
> > > > > - unset_icon_image(icon); */
> > > > > + unset_icon_image(icon);
> > > > > +
> > > > > + /* Set the new info */
> > > > > + icon->file = NULL;
> > > > > icon->file_image = image;
> > > >
> > > > But why setting icon->file to NULL helps? What goes wrong otherwise?
> > >
> > > Really I don't have idea, but now is fine. Problem in wfree()?
> > >
> > > See that wfree sets it to NULL. Needs the "if (ptr)" a bracket?
> > >
> > >
> > > void wfree(void *ptr)
> > > {
> > > if (ptr)
> > > #ifdef USE_BOEHM_GC
> > > /* This should eventually be removed, once the criss-cross
> > > * of wmalloc()d memory being free()d, malloc()d memory
> > > being
> > > * wfree()d, various misuses of calling wfree() on objects
> > > * allocated by libc malloc() and calling libc free() on
> > > * objects allocated by Boehm GC (think external
> > > libraries)
> > > * is cleaned up.
> > > */
> > > if (GC_base(ptr) != 0)
> > > GC_FREE(ptr);
> > > else
> > > free(ptr);
> > > #else
> > > free(ptr);
> > > #endif
> > > ptr = NULL;
> > > }
> >
> > I guess the bracket is not strictly necessary here under the rules,
> > but it's better to have it because it's fragile otherwise.
> >
> > I still don't see why your later patch is fine, but I haven't read
> > the code. I was hoping it would be clear to you so that's why I asked.
> >
> > Since I don't want to think about all patches in detail, I try to
> > enforce the rule "try to look like you know what you're doing" in the
> > changelog. So that's why changelogs are important, I want to understand
> > what's going on with minimal effort :-)
>
> Hi Carlos, Hi Rodolfo,
>
> From my understanding of the problem, it is because when the memory is freed
> the first time the pointer is kept to the freed location, and then it looks
> like the memory is still there, so a new call to wfree is possible.
>
> When setting the pointer to NULL, this actually stores the information that
> this memory is not reserved, so things works better. However the workaround
> proposed is not great because the pointer reset is not done at the right
> place. I'll send a patch to propose a different solution which I think should
> be safer.
Thanks.
> >
> > So when I read your email saying that it works, I want to know why.
> > Otherwise many monkeys might be typing stuff and at some point things
> > work too. No offense intended! I just want to emphasize the point :-)
> >
> > Perhaps this is all obvious and I'm embarrassing myself. But I don't
> > mind (too much).
> >
> >
> > --
> > To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
>
>
> --
> To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[PATCH] Added reset of pointer after memory free to avoid double-free crash
From 0bdc1cabc08f612b4a29f03e50051a15f2acf71c Mon Sep 17 00:00:00 2001
From: Christophe CURIS
Date: Wed, 14 Nov 2012 21:23:32 +0100
Subject: [PATCH] Added reset of pointer after memory free to avoid
double-free crash
As reported by Rodolfo, there are some cases when working with icons
where a crash can occur, which is related to trying to re-release
some memory that have been already freed previously.
This patch adds a reset-to-NULL of the corresponding pointers so that
on next usage wmaker will know there's no more memory associated with
these pointers.
---
src/icon.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/icon.c b/src/icon.c
index ba059ad..04767d8 100644
--- a/src/icon.c
+++ b/src/icon.c
@@ -584,11 +584,15 @@ void wIconSelect(WIcon * icon)
static void unset_icon_image(WIcon *icon)
{
- if (icon->file)
+ if (icon->file) {
wfree(icon->file);
+ icon->file = NULL;
+ }
- if (icon->file_image)
+ if (icon->file_image) {
RReleaseImage(icon->file_image);
+ icon->file_image = NULL;
+ }
}
void wIconUpdate(WIcon *icon)
@@ -755,8 +759,7 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
if (!image)
return 1;
- /* FIXME: If unset_icon_image, pointer double free then crash
- unset_icon_image(icon); */
+ unset_icon_image(icon);
icon->file_image = image;
return 0;
--
1.7.10.4
Re : Re: Help with wfree
- Carlos R. Mafra a écrit :
> On Wed, 14 Nov 2012 at 20:18:11 +0100, Rodolfo García Peñas wrote:
> > On Wed, 14 Nov 2012, Carlos R. Mafra escribió:
> >
> > > On Wed, 14 Nov 2012 at 19:34:42 +0100, Rodolfo García Peñas wrote:
> > > > Ok,
> > > >
> > > > seems to be ok now :-)
> > > >
> > > > --- a/src/icon.c
> > > > +++ b/src/icon.c
> > > > @@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon
> > > > *icon)
> > > > /* Resize the icon to the wPreferences.icon_size size */
> > > > image = wIconValidateIconSize(image, wPreferences.icon_size);
> > > >
> > > > - /* FIXME: If unset_icon_image, pointer double free then crash
> > > > - unset_icon_image(icon); */
> > > > + unset_icon_image(icon);
> > > > +
> > > > + /* Set the new info */
> > > > + icon->file = NULL;
> > > > icon->file_image = image;
> > >
> > > But why setting icon->file to NULL helps? What goes wrong otherwise?
> >
> > Really I don't have idea, but now is fine. Problem in wfree()?
> >
> > See that wfree sets it to NULL. Needs the "if (ptr)" a bracket?
> >
> >
> > void wfree(void *ptr)
> > {
> > if (ptr)
> > #ifdef USE_BOEHM_GC
> > /* This should eventually be removed, once the criss-cross
> > * of wmalloc()d memory being free()d, malloc()d memory
> > being
> > * wfree()d, various misuses of calling wfree() on objects
> > * allocated by libc malloc() and calling libc free() on
> > * objects allocated by Boehm GC (think external libraries)
> > * is cleaned up.
> > */
> > if (GC_base(ptr) != 0)
> > GC_FREE(ptr);
> > else
> > free(ptr);
> > #else
> > free(ptr);
> > #endif
> > ptr = NULL;
> > }
>
> I guess the bracket is not strictly necessary here under the rules,
> but it's better to have it because it's fragile otherwise.
>
> I still don't see why your later patch is fine, but I haven't read
> the code. I was hoping it would be clear to you so that's why I asked.
>
> Since I don't want to think about all patches in detail, I try to
> enforce the rule "try to look like you know what you're doing" in the
> changelog. So that's why changelogs are important, I want to understand
> what's going on with minimal effort :-)
Hi Carlos, Hi Rodolfo,
>From my understanding of the problem, it is because when the memory is freed
>the first time the pointer is kept to the freed location, and then it looks
>like the memory is still there, so a new call to wfree is possible.
When setting the pointer to NULL, this actually stores the information that
this memory is not reserved, so things works better. However the workaround
proposed is not great because the pointer reset is not done at the right place.
I'll send a patch to propose a different solution which I think should be safer.
>
> So when I read your email saying that it works, I want to know why.
> Otherwise many monkeys might be typing stuff and at some point things
> work too. No offense intended! I just want to emphasize the point :-)
>
> Perhaps this is all obvious and I'm embarrassing myself. But I don't
> mind (too much).
>
>
> --
> To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: Help with wfree
On Wed, 14 Nov 2012 at 20:18:11 +0100, Rodolfo García Peñas wrote:
> On Wed, 14 Nov 2012, Carlos R. Mafra escribió:
>
> > On Wed, 14 Nov 2012 at 19:34:42 +0100, Rodolfo García Peñas wrote:
> > > Ok,
> > >
> > > seems to be ok now :-)
> > >
> > > --- a/src/icon.c
> > > +++ b/src/icon.c
> > > @@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
> > > /* Resize the icon to the wPreferences.icon_size size */
> > > image = wIconValidateIconSize(image, wPreferences.icon_size);
> > >
> > > - /* FIXME: If unset_icon_image, pointer double free then crash
> > > - unset_icon_image(icon); */
> > > + unset_icon_image(icon);
> > > +
> > > + /* Set the new info */
> > > + icon->file = NULL;
> > > icon->file_image = image;
> >
> > But why setting icon->file to NULL helps? What goes wrong otherwise?
>
> Really I don't have idea, but now is fine. Problem in wfree()?
>
> See that wfree sets it to NULL. Needs the "if (ptr)" a bracket?
>
>
> void wfree(void *ptr)
> {
> if (ptr)
> #ifdef USE_BOEHM_GC
> /* This should eventually be removed, once the criss-cross
> * of wmalloc()d memory being free()d, malloc()d memory being
> * wfree()d, various misuses of calling wfree() on objects
> * allocated by libc malloc() and calling libc free() on
> * objects allocated by Boehm GC (think external libraries)
> * is cleaned up.
> */
> if (GC_base(ptr) != 0)
> GC_FREE(ptr);
> else
> free(ptr);
> #else
> free(ptr);
> #endif
> ptr = NULL;
> }
I guess the bracket is not strictly necessary here under the rules,
but it's better to have it because it's fragile otherwise.
I still don't see why your later patch is fine, but I haven't read
the code. I was hoping it would be clear to you so that's why I asked.
Since I don't want to think about all patches in detail, I try to
enforce the rule "try to look like you know what you're doing" in the
changelog. So that's why changelogs are important, I want to understand
what's going on with minimal effort :-)
So when I read your email saying that it works, I want to know why.
Otherwise many monkeys might be typing stuff and at some point things
work too. No offense intended! I just want to emphasize the point :-)
Perhaps this is all obvious and I'm embarrassing myself. But I don't
mind (too much).
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: Help with wfree
On Wed, 14 Nov 2012, Carlos R. Mafra escribió:
> On Wed, 14 Nov 2012 at 19:34:42 +0100, Rodolfo García Peñas wrote:
> > Ok,
> >
> > seems to be ok now :-)
> >
> > --- a/src/icon.c
> > +++ b/src/icon.c
> > @@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
> > /* Resize the icon to the wPreferences.icon_size size */
> > image = wIconValidateIconSize(image, wPreferences.icon_size);
> >
> > - /* FIXME: If unset_icon_image, pointer double free then crash
> > - unset_icon_image(icon); */
> > + unset_icon_image(icon);
> > +
> > + /* Set the new info */
> > + icon->file = NULL;
> > icon->file_image = image;
>
> But why setting icon->file to NULL helps? What goes wrong otherwise?
Really I don't have idea, but now is fine. Problem in wfree()?
See that wfree sets it to NULL. Needs the "if (ptr)" a bracket?
void wfree(void *ptr)
{
if (ptr)
#ifdef USE_BOEHM_GC
/* This should eventually be removed, once the criss-cross
* of wmalloc()d memory being free()d, malloc()d memory being
* wfree()d, various misuses of calling wfree() on objects
* allocated by libc malloc() and calling libc free() on
* objects allocated by Boehm GC (think external libraries)
* is cleaned up.
*/
if (GC_base(ptr) != 0)
GC_FREE(ptr);
else
free(ptr);
#else
free(ptr);
#endif
ptr = NULL;
}
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: Help with wfree
On Wed, 14 Nov 2012 at 19:34:42 +0100, Rodolfo García Peñas wrote: > Ok, > > seems to be ok now :-) > > --- a/src/icon.c > +++ b/src/icon.c > @@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon) > /* Resize the icon to the wPreferences.icon_size size */ > image = wIconValidateIconSize(image, wPreferences.icon_size); > > - /* FIXME: If unset_icon_image, pointer double free then crash > - unset_icon_image(icon); */ > + unset_icon_image(icon); > + > + /* Set the new info */ > + icon->file = NULL; > icon->file_image = image; But why setting icon->file to NULL helps? What goes wrong otherwise? -- To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Re: Help with wfree
Ok,
seems to be ok now :-)
--- a/src/icon.c
+++ b/src/icon.c
@@ -778,8 +778,10 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
/* Resize the icon to the wPreferences.icon_size size */
image = wIconValidateIconSize(image, wPreferences.icon_size);
- /* FIXME: If unset_icon_image, pointer double free then crash
- unset_icon_image(icon); */
+ unset_icon_image(icon);
+
+ /* Set the new info */
+ icon->file = NULL;
icon->file_image = image;
return 0;
On Wed, 14 Nov 2012, Rodolfo kix Garcia escribió:
> Hi,
>
> I don't understand why this happends. If I do this change:
>
> @@ -778,8 +778,7 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
> /* Resize the icon to the wPreferences.icon_size size */
> image = wIconValidateIconSize(image, wPreferences.icon_size);
>
> - /* FIXME: If unset_icon_image, pointer double free then crash
> - unset_icon_image(icon); */
> + unset_icon_image(icon);
> icon->file_image = image;
>
> wmaker crash by double free or corruption. The problem is only with
> get_rimage_icon_from_wm_hints(), but unset_icon_image() is used in multiple
> functions.
>
> If I do this change, no problem:
>
> @@ -591,8 +591,8 @@ void wIconSelect(WIcon * icon)
>
> static void unset_icon_image(WIcon *icon)
> {
> - if (icon->file)
> - wfree(icon->file);
> +// if (icon->file)
> +// wfree(icon->file);
>
> But of course, icon->file is not freeded.
>
> Please, help.
>
> Thanks.
> kix
> --
> ||// //\\// Rodolfo "kix" Garcia
> ||\\// //\\ http://www.kix.es/
>
>
> --
> To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
Help with wfree
Hi,
I don't understand why this happends. If I do this change:
@@ -778,8 +778,7 @@ static int get_rimage_icon_from_wm_hints(WIcon *icon)
/* Resize the icon to the wPreferences.icon_size size */
image = wIconValidateIconSize(image, wPreferences.icon_size);
- /* FIXME: If unset_icon_image, pointer double free then crash
- unset_icon_image(icon); */
+ unset_icon_image(icon);
icon->file_image = image;
wmaker crash by double free or corruption. The problem is only with
get_rimage_icon_from_wm_hints(), but unset_icon_image() is used in multiple
functions.
If I do this change, no problem:
@@ -591,8 +591,8 @@ void wIconSelect(WIcon * icon)
static void unset_icon_image(WIcon *icon)
{
- if (icon->file)
- wfree(icon->file);
+// if (icon->file)
+// wfree(icon->file);
But of course, icon->file is not freeded.
Please, help.
Thanks.
kix
--
||// //\\// Rodolfo "kix" Garcia
||\\// //\\ http://www.kix.es/
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
[repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.3-182-g92d5252
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project wmaker-crm.git.
The branch, next has been updated
via 92d52523c411d10be08e3af28fc5ac924b2f5075 (commit)
from eb51844fa8742bedd0f0ede5f89762963689e095 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -
http://repo.or.cz/w/wmaker-crm.git/commit/92d52523c411d10be08e3af28fc5ac924b2f5075
commit 92d52523c411d10be08e3af28fc5ac924b2f5075
Author: BALATON Zoltan
Date: Wed Nov 14 01:24:06 2012 +0100
WPrefs: Move around some options between pages
Move bounce options from expert prefs to its own box on ergonomic
prefs to have them at one place which makes them somewhat more clear.
The options previously occupying this place have been moved to other
pages where they better belong.
diff --git a/WPrefs.app/Expert.c b/WPrefs.app/Expert.c
index b14f549..5d539fc 100644
--- a/WPrefs.app/Expert.c
+++ b/WPrefs.app/Expert.c
@@ -21,6 +21,12 @@
#include "WPrefs.h"
+#ifdef XKB_MODELOCK
+#define NUMITEMS 12
+#else
+#define NUMITEMS 11
+#endif
+
typedef struct _Panel {
WMBox *box;
char *sectionName;
@@ -31,7 +37,7 @@ typedef struct _Panel {
WMWidget *parent;
- WMButton *swi[14];
+ WMButton *swi[NUMITEMS];
} _Panel;
@@ -51,10 +57,10 @@ static void showData(_Panel * panel)
WMSetButtonSelected(panel->swi[7], GetBoolForKey("SingleClickLaunch"));
WMSetButtonSelected(panel->swi[8],
GetBoolForKey("CycleActiveHeadOnly"));
WMSetButtonSelected(panel->swi[9], GetBoolForKey("ShowClipTitle"));
- WMSetButtonSelected(panel->swi[10],
GetBoolForKey("BounceAppIconsWhenUrgent"));
- WMSetButtonSelected(panel->swi[11],
GetBoolForKey("RaiseAppIconsWhenBouncing"));
- WMSetButtonSelected(panel->swi[12],
GetBoolForKey("OpaqueMoveResizeKeyboard"));
- WMSetButtonSelected(panel->swi[13],
GetBoolForKey("DoNotMakeAppIconsBounce"));
+ WMSetButtonSelected(panel->swi[10],
GetBoolForKey("OpaqueMoveResizeKeyboard"));
+#ifdef XKB_MODELOCK
+ WMSetButtonSelected(panel->swi[11], GetBoolForKey("KbdModeLock"));
+#endif /* XKB_MODELOCK */
}
static void createPanel(Panel * p)
@@ -75,10 +81,10 @@ static void createPanel(Panel * p)
WMSetScrollViewHasHorizontalScroller(sv, False);
f = WMCreateFrame(panel->box);
- WMResizeWidget(f, 495, sizeof(panel->swi) / sizeof(char *) * 25 + 8);
+ WMResizeWidget(f, 495, NUMITEMS * 25 + 8);
WMSetFrameRelief(f, WRFlat);
- for (i = 0; i < sizeof(panel->swi) / sizeof(char *); i++) {
+ for (i = 0; i < NUMITEMS; i++) {
panel->swi[i] = WMCreateSwitchButton(f);
WMResizeWidget(panel->swi[i], FRAME_WIDTH - 40, 25);
WMMoveWidget(panel->swi[i], 5, 5 + i * 25);
@@ -95,15 +101,14 @@ static void createPanel(Panel * p)
WMSetButtonText(panel->swi[7], _("Launch applications and restore
windows with a single click."));
WMSetButtonText(panel->swi[8], _("Cycle windows only on the active
head."));
WMSetButtonText(panel->swi[9], _("Show workspace title on Clip."));
- WMSetButtonText(panel->swi[10], _("Bounce AppIcons when the application
wants attention."));
- WMSetButtonText(panel->swi[11], _("Raise AppIcons when bouncing."));
- WMSetButtonText(panel->swi[12], _("Opaque Move,Resize with keyboard."));
- WMSetButtonText(panel->swi[13], _("Do not make AppIcons bounce."));
+ WMSetButtonText(panel->swi[10], _("Opaque Move,Resize with keyboard."));
+#ifdef XKB_MODELOCK
+ WMSetButtonText(panel->swi[11], _("Enable keyboard language switch
button in window titlebars."));
+#endif /* XKB_MODELOCK */
/* If the item is default true, enable the button here */
WMSetButtonEnabled(panel->swi[6], True);
WMSetButtonEnabled(panel->swi[9], True);
- WMSetButtonEnabled(panel->swi[10], True);
WMMapSubwidgets(panel->box);
WMSetScrollViewContentView(sv, WMWidgetView(f));
@@ -128,10 +133,10 @@ static void storeDefaults(_Panel * panel)
SetBoolForKey(WMGetButtonSelected(panel->swi[7]), "SingleClickLaunch");
SetBoolForKey(WMGetButtonSelected(panel->swi[8]),
"CycleActiveHeadOnly");
SetBoolForKey(WMGetButtonSelected(panel->swi[9]), "ShowClipTitle");
- SetBoolForKey(WMGetButtonSelected(panel->swi[10]),
"BounceAppIconsWhenUrgent");
- SetBoolForKey(WMGetButtonSelected(panel->swi[11]),
"RaiseAppIconsWhenBouncing");
- SetBoolForKey(WMGetButtonSelected(panel->swi[12]),
"OpaqueMoveResizeKeyboard");
- SetBoolForKey(WMGetButtonSelected(panel->swi[13]),
"DoNotMakeAppIconsBounce");
+ SetBoolForKey(WMGetButtonSelected(panel->swi[10]),
[repo.or.cz] wmaker-crm.git branch next updated: wmaker-0.95.3-183-g5dcd31a
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project wmaker-crm.git.
The branch, next has been updated
via 5dcd31acbe94959656e4d971c0f4b462a0bfb7c7 (commit)
from 92d52523c411d10be08e3af28fc5ac924b2f5075 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -
http://repo.or.cz/w/wmaker-crm.git/commit/5dcd31acbe94959656e4d971c0f4b462a0bfb7c7
commit 5dcd31acbe94959656e4d971c0f4b462a0bfb7c7
Author: Carlos R. Mafra
Date: Wed Nov 14 10:20:27 2012 +
appicon: Avoid double 'Hide' entry
On 12.11.2012 Paul Seelig reported:
- open an application positioning an app icon on the bottom
- right click this app icon to show the context menu
- wonder yourself why there are two lines saying "Hide"
- first Hide entry does not do anything, second does
The reason for this curious behavior is the following.
The "Launch" entry was added in 8352c9ef60 ("Allow relaunch with shortcut
key")
as the first one in the appicon menu, but this first position was hard-coded
in another part of wmaker's code in order to decide the menu entry text
based
on the application's 'hidden' state in openApplicationMenu():
if (wapp->flags.hidden)
menu->entries[1]->text = _("Unhide");
else
menu->entries[1]->text = _("Hide");
But the "Launch" entry is before these "Hide/Unhide" entries and now the
assumption
about entries[1] containing the relevant string for this hide/unhide
decision is
no longer valid.
The simpler "fix" is to move the "Launch" entry below these "Hide/Unhide"
games.
diff --git a/src/appicon.c b/src/appicon.c
index 329bd6e..ac0c4ba 100644
--- a/src/appicon.c
+++ b/src/appicon.c
@@ -577,9 +577,9 @@ static WMenu *createApplicationMenu(WScreen *scr)
WMenu *menu;
menu = wMenuCreate(scr, NULL, False);
- wMenuAddCallback(menu, _("Launch"), relaunchCallback, NULL);
wMenuAddCallback(menu, _("Unhide Here"), unhideHereCallback, NULL);
wMenuAddCallback(menu, _("Hide"), hideCallback, NULL);
+ wMenuAddCallback(menu, _("Launch"), relaunchCallback, NULL);
wMenuAddCallback(menu, _("Set Icon..."), setIconCallback, NULL);
wMenuAddCallback(menu, _("Kill"), killCallback, NULL);
---
Summary of changes:
src/appicon.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
repo.or.cz automatic notification. Contact project admin crma...@gmail.com
if you want to unsubscribe, or site admin ad...@repo.or.cz if you receive
no reply.
--
wmaker-crm.git ("The Window Maker window manager")
--
To unsubscribe, send mail to wmaker-dev-unsubscr...@lists.windowmaker.org.
