[PATCH 1/3] Delete redundant scrnum field from Xvfb private screen-info struct.

Jamey Sharp jamey at minilop.net
Wed Apr 28 09:54:19 PDT 2010


On Tue, Apr 27, 2010 at 01:07:02PM +0300, Tiago Vignatti wrote:
> On Tue, Apr 27, 2010 at 04:17:10AM +0200, ext Jamey Sharp wrote:
> ...
> 
> > -    sprintf(pvfb->mmap_file, "%s/Xvfb_screen%d", pfbdir, pvfb->scrnum);
> > +    sprintf(pvfb->mmap_file, "%s/Xvfb_screen%d", pfbdir, (int) (pvfb - vfbScreens));
> 
> ...
> 
> > -    ErrorF("screen %d shmid %d\n", pvfb->scrnum, pvfb->shmid);
> > +    ErrorF("screen %d shmid %d\n", (int) (pvfb - vfbScreens), pvfb->shmid);
> 
> That's a really nice catch :) But hey, do we need to be verbose here? I'd
> prefer to avoid these pointer calculation, which is really bad for readability
> of code.

Both of those statements are necessary, I believe. The first ensures
that each mmap'd framebuffer has a different name and the user can work
out which one came from which screen. The second looks like the only way
to find out which shared-memory segment the server allocated for each
screen. (Both modes are disabled by default, enabled by command line
options.)

On Tue, Apr 27, 2010 at 07:30:15AM -0700, Alan Coopersmith wrote:
> That could be avoided by just passing scrnum down as an argument through the
> vfbAllocate* functions, since it's an argument (index) to the vfbScreenInit()
> that calls vfbAllocateFramebufferMemory() in the first place.

Good point. That's a rather larger change than I wanted to make, though,
particularly since I'm hoping to replace Xfake and Xvfb with small shell
scripts using the video-dummy driver, eventually. :-)

Jamey


More information about the xorg-devel mailing list