On Tue, 26 Jun 2012 at 10:32:09 +0200, Rodolfo kix Garcia wrote:
> El 26.06.2012 01:12, Carlos R. Mafra escribió:
> >On Mon, 25 Jun 2012 at 23:48:51 +0200, Rodolfo García Peñas wrote:
> >>
> >>Subject: [PATCH 07/11] net_state_from_client can be removed
> >>
> >>The variable net_state_from_client don't change their value in the
> >>code, therefore, we don't need check it in any "if".
> >>---
> >> src/window.h | 1 -
> >> src/wmspec.c | 4 +---
> >> 2 files changed, 1 insertion(+), 4 deletions(-)
> >>
> >>diff --git a/src/window.h b/src/window.h
> >>index e2d516a..ae1e704 100644
> >>--- a/src/window.h
> >>+++ b/src/window.h
> >>@@ -290,7 +290,6 @@ typedef struct WWindow {
> >>
> >> unsigned int user_changed_width:1;
> >> unsigned int user_changed_height:1;
> >>- unsigned int net_state_from_client:1; /* state hint was
> >>set by client */
> >> unsigned int net_skip_pager:1;
> >> unsigned int net_handle_icon:1;
> >> unsigned int net_show_desktop:1;
> >>diff --git a/src/wmspec.c b/src/wmspec.c
> >>index 9456841..ae870ea 100644
> >>--- a/src/wmspec.c
> >>+++ b/src/wmspec.c
> >>@@ -795,9 +795,7 @@ static void updateWorkspaceHint(WWindow *
> >>wwin, Bool fake, Bool del)
> >> static void updateStateHint(WWindow * wwin, Bool
> >>changedWorkspace, Bool del)
> >> { /* changeable */
> >> if (del) {
> >>- if (!wwin->flags.net_state_from_client) {
> >>- XDeleteProperty(dpy, wwin->client_win, net_wm_state);
> >>- }
> >>+ XDeleteProperty(dpy, wwin->client_win, net_wm_state);
> >
> >I don't know. I wonder if we should try to look where and when
> >we should set this variable.
>
> I had the same question. Can I remove this?
>
> net_state_from_client is defined in the struct, but never is
> initalized. Then, is used in the "if".
> Normally, the compiler can init a value to 0 (or not!). Then, this
> could be an error.
>
> What I did? I added a printfs, something like:
>
> if (del) {
> printf("1\n");
> if (!wwin->flags.net_state_from_client) {
> printf("2\n");
> XDeleteProperty(dpy, wwin->client_win, net_wm_state);
> }
>
> And then I runned windowmaker some time. Open windows,... I always
> get "1\n2\n" on the screen. If I got a 1, then I got a 2. That make
> sense, because the value of wwin->flags.net_state_from_client should
> be "0" (compiler init), then "if (!0)" is like "true".
>
> For this reason, I remove the "if".
You should have written this in the changelog in the first place.
That's a much better description of what's going on and it helps
making a better decision whether to apply it or not.
Do not avoid being verbose about what led you to write a patch,
and the testing you've done. It helps me to understand. So consider
writing more friendly (in the sense of teaching) commit messages
as a way to help me understand the issues after a day of long long
long calculations. I'd appreciate that.
> I any case, we should do something. Now the value depends in if the
> compiler init or not the value. Do you like this code?
>
> int main(void)
> {
> int a;
>
> if (!a)
> printf("No a\n");
> }
Yeah, the code above is not good. The compiler warns me but I do
get the "No a" message.
> The compiler will show a warning, because "a" is not initialized.
> IMO, is the same case (I'm the compiler ;-) ).
Hm, gcc never warned me about flags.net_state_from_client being
uninitialized. I wonder why.
But I guess that in this case removing the if() is the correct thing
to do.
The patch is already applied.
--
To unsubscribe, send mail to [email protected].
