Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FScreenGetScrRect function #163

Closed
klebertarcisio opened this issue Jul 9, 2020 · 5 comments
Closed

FScreenGetScrRect function #163

klebertarcisio opened this issue Jul 9, 2020 · 5 comments

Comments

@klebertarcisio
Copy link
Contributor

klebertarcisio commented Jul 9, 2020

Hi everyone,

I would like to understand a little more about the FScreenGetScrRect function:

fvwm3/libs/FScreen.c

Lines 636 to 656 in 48dd509

Bool FScreenGetScrRect(fscreen_scr_arg *arg, fscreen_scr_t screen,
int *x, int *y, int *w, int *h)
{
struct monitor *m = FindScreen(arg, screen);
if (m == NULL) {
fvwm_debug(__func__, "%s: m is NULL\n", __func__);
return (True);
}
if (x)
*x = m->si->x;
if (y)
*y = m->si->y;
if (w)
*w = m->si->w;
if (h)
*h = m->si->h;
return !((monitor_get_count() > 1) &&
(strcmp(m->si->name, GLOBAL_SCREEN_NAME) == 0));
}

This function may not change the values of x, y, w and h. That way, I always have to pass x, y, w and h initialized, right? If I don't pass these initialized values, I can't guarantee that the function initialized them unless I check the function's return, but that return is hardly used in the code.

See this example:

fvwm3/fvwm/icons.c

Lines 2041 to 2058 in 48dd509

{
/* if first time */
int sx;
int sy;
int sw;
int sh;
/* Right now, the global box is hard-coded, fills the primary
* screen, uses an 80x80 grid, and fills top-bottom,
* left-right */
FScreenGetScrRect(NULL, FSCREEN_PRIMARY, &sx, &sy, &sw, &sh);
global_icon_box_ptr = fxcalloc(1, sizeof(icon_boxes));
global_icon_box_ptr->IconBox[0] = sx;
global_icon_box_ptr->IconBox[1] = sy;
global_icon_box_ptr->IconBox[2] = sx + sw;
global_icon_box_ptr->IconBox[3] = sy + sh;
global_icon_box_ptr->IconGrid[0] = 80;
global_icon_box_ptr->IconGrid[1] = 80;
global_icon_box_ptr->IconFlags = ICONFILLHRZ;

Is there any possibility that global_icon_box_ptr-> IconBox [0] will receive an uninitialized value?

@issue-label-bot
Copy link

Issue Label Bot is not confident enough to auto-label this issue. See dashboard for more details.

@ThomasAdam
Copy link
Member

Hi @klebertarcisio,

Yes, there's a small risk, but that's not the point here, because:

  • Most compilers will set the value to 0 if undeclared in this case;
  • The context for these callers is such that it's very unlikely we're going to fail finding a screen to set the values in the first place.

@danespen
Copy link

danespen commented Jul 9, 2020 via email

@ThomasAdam
Copy link
Member

Hi @danespen,

Yes -- that's still the case here; per screen iconboxes can be definedm and come and go depending on whether the monitor is attached or not. The PRIMARY monitor is now always set regardless.

@klebertarcisio
Copy link
Contributor Author

klebertarcisio commented Jul 9, 2020

In order to contribute to this discussion, I will post a few more places that the FScreenGetScrRect function is used in a similar way.

FScreen.c

fvwm3/libs/FScreen.c

Lines 661 to 684 in 48dd509

void FScreenTranslateCoordinates(
fscreen_scr_arg *arg_src, fscreen_scr_t screen_src,
fscreen_scr_arg *arg_dest, fscreen_scr_t screen_dest,
int *x, int *y)
{
int x_src;
int y_src;
int x_dest;
int y_dest;
FScreenGetScrRect(arg_src, screen_src, &x_src, &y_src, NULL, NULL);
FScreenGetScrRect(arg_dest, screen_dest, &x_dest, &y_dest, NULL, NULL);
if (x)
{
*x = *x + x_src - x_dest;
}
if (y)
{
*y = *y + y_src - y_dest;
}
return;
}

FvwmIdent.c

{
int sx;
int sy;
int sw;
int sh;
Window JunkW;
int JunkC;
unsigned int JunkM;
fscreen_scr_arg fscr;
if (!FQueryPointer(
dpy, Root, &JunkW, &JunkW, &x, &y, &JunkC, &JunkC,
&JunkM))
{
/* pointer is on a different screen */
x = 0;
y = 0;
}
fscr.xypos.x = x;
fscr.xypos.y = y;
FScreenGetScrRect(&fscr, FSCREEN_XYPOS, &sx, &sy, &sw, &sh);
if (y + height + 100 > sy + sh)
{
y = sy + sh - height - 10;
mysizehints.win_gravity = SouthWestGravity;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants