Re: [edid-decode] [PATCH 2/2] Calculate DisplayID checksums. Refactor do_checksum.
Am 10.12.2016 20:44, schrieb Mark Ferry:
> ---
> edid-decode.c | 40 +---
> 1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index c18697f..6df2b6e 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -47,6 +47,7 @@ static int has_range_descriptor = 0;
> static int has_preferred_timing = 0;
> static int has_valid_checksum = 1;
> static int has_valid_cvt = 1;
> +static int has_valid_displayid_checksum = 1;
> static int has_valid_dummy_block = 1;
> static int has_valid_week = 0;
> static int has_valid_year = 0;
> @@ -560,23 +561,27 @@ detailed_block(unsigned char *x, int in_extension)
> return 1;
> }
>
> -static void
> -do_checksum(unsigned char *x)
> +static unsigned char
> +do_checksum(unsigned char *x, size_t len)
> {
> -printf("Checksum: 0x%hx", x[0x7f]);
> -{
> - unsigned char sum = 0;
> - int i;
> - for (i = 0; i < 128; i++)
> - sum += x[i];
> - if (sum) {
> - printf(" (should be 0x%hx)", (unsigned char)(x[0x7f] - sum));
> - has_valid_checksum = 0;
> - } else printf(" (valid)");
> -}
> +unsigned char sum = 0;
> +int i;
> +
> +printf("Checksum: 0x%hx", x[len -1]);
> +
> +for (i = 0; i < len; i++)
> +sum += x[i];
> +
> +if (sum) {
> +printf(" (should be 0x%hx)", (unsigned char)(x[len-1] - sum));
> +} else printf(" (valid)");
> +
> printf("\n");
any reason not to move the \n to the printf in front ?
just thinking of it ..
perhaps this can be reduced to invalid/valid ? i do not know anyone
seeing more in that information.
then you can simply return 0/-1.
> +
> +return sum;
> }
>
> +
> /* CEA extension */
>
> static const char *
> @@ -1281,7 +1286,7 @@ parse_cea(unsigned char *x)
> detailed_block(detailed, 1);
> } while (0);
>
> -do_checksum(x);
> +(void) do_checksum(x, 128);
>
> return ret;
> }
> @@ -1371,6 +1376,9 @@ parse_displayid(unsigned char *x)
> int ext_count = x[4];
> int i;
> printf("Length %d, version %d, extension count %d\n", length, version,
> ext_count);
> +
> +has_valid_displayid_checksum = (do_checksum(x+1, length + 5) == 0x0);
> +
> int offset = 5;
> while (length > 0) {
> int tag = x[offset];
> @@ -2037,7 +2045,7 @@ int main(int argc, char **argv)
> has_valid_extension_count = 1;
> }
>
> -do_checksum(edid);
> +(void) do_checksum(edid, 128);
>
in General i am a fan of checking return values,
why not here ?
and why the magic 128 ?
Perhaps we can avoid the global ?
hope that helps,
re,
wh
> x = edid;
> for (edid_lines /= 8; edid_lines > 1; edid_lines--) {
> @@ -2127,6 +2135,8 @@ int main(int argc, char **argv)
> printf("\tInvalid detailed timing descriptor ordering\n");
> if (!has_valid_range_descriptor)
> printf("\tRange descriptor contains garbage\n");
> + if (!has_valid_displayid_checksum)
> + printf("\tBlock has broken DisplayID checksum\n");
> if (!has_valid_max_dotclock)
> printf("\tEDID 1.4 block does not set max dotclock\n");
> }
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [edid-decode] [PATCH 2/2] Calculate DisplayID checksums. Refactor do_checksum.
Am 13.12.2016 11:39, schrieb Mark Ferry:
> Thanks for the feedback Walter. Comments below.
>
> On Mon, 12 Dec 2016 20:02:08 +0100, walter harms wrote:
>>
>>
>> Am 10.12.2016 20:44, schrieb Mark Ferry:
>>> +printf("Checksum: 0x%hx", x[len -1]);
>>> +
>>> +for (i = 0; i < len; i++)
>>> +sum += x[i];
>>> +
>>> +if (sum) {
>>> +printf(" (should be 0x%hx)", (unsigned char)(x[len-1] - sum));
>>> +} else printf(" (valid)");
>>> +
>>> printf("\n");
>>
>> any reason not to move the \n to the printf in front ?
>> just thinking of it ..
>> perhaps this can be reduced to invalid/valid ? i do not know anyone
>> seeing more in that information.
>> then you can simply return 0/-1.
>>
>
> Agree about the newline and a boolean return value.
>
> I definitely want to keep the "should be 0xxx" output.
> Hacking EDIDs and having to recalculate the checksum myself is what
> motivated this patch. Not sure if that's what you meant.
But your are printing (x[len-1] - sum), where x[len-1]== checksum
perhaps you wanted to print -sum ? (I assume the "checksum" is just
the diff to 0).
INHO the code should be like this:
exp_chk= x[len -1]; // expected sum
for (i = 0; i < len-1; i++) sum += x[i]; // real sum, notice len-1
if (exp_chk+sum != 0 )
printf("expected %02x found 0x02x\"n, 255-exp_chk, sum);
/*
i have no idea about EDID checksum nor tested the code, take with a pinch of
salt
*/
re,
wh
>
>>>
>>> -do_checksum(edid);
>>> +(void) do_checksum(edid, 128);
>>>
>> in General i am a fan of checking return values,
>> why not here ?
>
> Thanks you're quite right, and I've failed to set has_valid_checksum.
>
>> and why the magic 128 ?
>> Perhaps we can avoid the global ?
>>
>
> There are plenty of magic numbers throughout which I'm not about to
> clean up in this patch. But I'll take the opportunity to create
> EDID_PAGE_SIZE for 128.
>
> Updated patch to follow.
>
> - Mark
>
>
>
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:xdm] Install xdm man pages under admin section (8), not user programs (1)
Am 17.01.2017 01:26, schrieb Alan Coopersmith: > As best I can tell, it was historically under section 1 mainly because > the old X Consortium Imake configs only supporting installing program > man pages there, and didn't have an option for using other sections. > > Signed-off-by: Alan Coopersmith > --- > > I'm sending this out not just for review that I made the right changes, > but to see if there's general agreement this change is right. It was > pointed out by one of our users who thought it odd that gdm's man page > is in section 8, but xdm's is in section 1. Yep, this is odd, GDM is wrong here. (from man (1)) 1 Executable programs or shell commands 8 System administration commands (usually only for root) The section 8 is intended for programms like mkfs,fsck an the other usual admin stuff, things you put in sbin. re, wh > > man/Makefile.am | 20 ++-- > man/xdm.man | 4 ++-- > man/xdmshell.man | 6 +++--- > 3 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/man/Makefile.am b/man/Makefile.am > index b71409c..2a42d89 100644 > --- a/man/Makefile.am > +++ b/man/Makefile.am > @@ -21,22 +21,22 @@ > # DEALINGS IN THE SOFTWARE. > # > > -appmandir = $(APP_MAN_DIR) > -appman_PRE = xdm.man > -appman_DATA = $(appman_PRE:man=$(APP_MAN_SUFFIX)) > +adminmandir = $(ADMIN_MAN_DIR) > +adminman_PRE = xdm.man > +adminman_DATA = $(adminman_PRE:man=$(ADMIN_MAN_SUFFIX)) > > -xdmlmandir = $(APP_MAN_DIR) > +xdmlmandir = $(ADMIN_MAN_DIR) > xdmlman_PRE = xdmshell.man > > if INSTALL_XDMSHELL > -xdmlman_DATA = $(xdmlman_PRE:man=$(APP_MAN_SUFFIX)) > +xdmlman_DATA = $(xdmlman_PRE:man=$(ADMIN_MAN_SUFFIX)) > else > -noinst_DATA = $(xdmlman_PRE:man=$(APP_MAN_SUFFIX)) > +noinst_DATA = $(xdmlman_PRE:man=$(ADMIN_MAN_SUFFIX)) > endif > > -EXTRA_DIST = $(appman_PRE) $(xdmlman_PRE) > -CLEANFILES = $(appman_DATA) $(xdmlman_DATA) $(noinst_DATA) > -SUFFIXES = .$(APP_MAN_SUFFIX) .man > +EXTRA_DIST = $(adminman_PRE) $(xdmlman_PRE) > +CLEANFILES = $(adminman_DATA) $(xdmlman_DATA) $(noinst_DATA) > +SUFFIXES = .$(ADMIN_MAN_SUFFIX) .man > > # String replacements in MAN_SUBSTS now come from xorg-macros.m4 via > configure > MAN_SUBSTS +=-e 's|CHOOSERPATH|$(XDMLIBDIR)/chooser|g' \ > @@ -51,5 +51,5 @@ MAN_SUBSTS += -e > 's|CHOOSERPATH|$(XDMLIBDIR)/chooser|g' \ > -e 's|DEV_RANDOM|$(DEV_RANDOM)|g' \ > -e 's|ARC4_RANDOM|$(HAVE_ARC4RANDOM)|g' > > -.man.$(APP_MAN_SUFFIX): > +.man.$(ADMIN_MAN_SUFFIX): > $(AM_V_GEN)$(SED) $(MAN_SUBSTS) < $< > $@ > diff --git a/man/xdm.man b/man/xdm.man > index 0648d71..ef57d8c 100644 > --- a/man/xdm.man > +++ b/man/xdm.man > @@ -23,7 +23,7 @@ > .\" from The Open Group. > .\" > .\" > -.TH XDM 1 __xorgversion__ > +.TH XDM __adminmansuffix__ __xorgversion__ > .SH NAME > xdm \- X Display Manager with support for XDMCP, host chooser > .SH SYNOPSIS > @@ -1454,7 +1454,7 @@ Kerberos credentials cache > .IR sessreg (__appmansuffix__), > .IR Xserver (__appmansuffix__), > .\" .IR chooser (__appmansuffix__), \" except that there isn't a manual for > it yet > -.IR xdmshell (__appmansuffix__), > +.IR xdmshell (__adminmansuffix__), > .IR fonts.conf (__filemansuffix__). > .br > .I "X Display Manager Control Protocol" > diff --git a/man/xdmshell.man b/man/xdmshell.man > index 8f27193..7bd683f 100644 > --- a/man/xdmshell.man > +++ b/man/xdmshell.man > @@ -44,7 +44,7 @@ > .\" DEALINGS IN THE SOFTWARE. > .\" > .\" > -.TH XDMSHELL __appmansuffix__ __xorgversion__ > +.TH XDMSHELL __adminmansuffix__ __xorgversion__ > .SH NAME > xdmshell \- shell for starting xdm on login > .SH SYNOPSIS > @@ -66,7 +66,7 @@ disable logins on that line until somebody types the > following as root: > .RE > .LP > On some platforms, one alternative is to disable logins on the console > -and always run \fIxdm\fP(__appmansuffix__) from \fI/etc/inittab\fP. > +and always run \fIxdm\fP(__adminmansuffix__) from \fI/etc/inittab\fP. > .LP > Another approach is to set up an account whose shell is the \fIxdmshell\fP > program found in the xdm distribution. This program is not installed by > @@ -104,5 +104,5 @@ login to the console directly. Whether or not this is > desirable depends on > the particular site. > .SH "SEE ALSO" > .IR X (__miscmansuffix__), > -.IR xdm (__appmansuffix__), > +.IR xdm (__adminmansuffix__), > .IR xinit (__appmansuffix__) ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libdrm v3 2/5] xf86drm: Add USB support
Am 18.01.2017 10:02, schrieb Thierry Reding:
> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
> infrastructure.
>
> v3:
> - guard Linux-specific sysfs parsing code with #ifdef __linux__
>
> v2:
> - make sysfs_uevent_get() more flexible using a format string
>
> Signed-off-by: Thierry Reding
> ---
> xf86drm.c | 175
> ++
> xf86drm.h | 13 +
> 2 files changed, 188 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 7766bfe937db..d83674e638c4 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2886,6 +2886,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
> return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
> }
>
> +#ifdef __linux__
> +static char * DRM_PRINTFLIKE(2, 3)
> +sysfs_uevent_get(const char *path, const char *fmt, ...)
> +{
> +char filename[PATH_MAX + 1], *key, *line = NULL, *value = NULL;
char *filename=NULL, *key, *line = NULL, *value = NULL;
> +size_t size = 0, len;
> +ssize_t num;
> +va_list ap;
> +FILE *fp;
> +
> +va_start(ap, fmt);
> +num = vasprintf(&key, fmt, ap);
> +va_end(ap);
> +len = num;
> +
> +snprintf(filename, sizeof(filename), "%s/uevent", path);
since asprintf() is available you could use:
asprintf(&filename,"%s/uevent", path);
same could be done for path below.
> +
> +fp = fopen(filename, "r");
> +if (!fp)
goto release_mem;
> +
> +while ((num = getline(&line, &size, fp)) >= 0) {
> +if ((strncmp(line, key, len) == 0) && (line[len] == '=')) {
> +char *start = line + len + 1, *end = line + num - 1;
> +
> +if (*end != '\n')
> +end++;
> +
> +value = strndup(start, end - start);
> +break;
> +}
> +}
> +
> +free(line);
> +fclose(fp);
> +
release_mem:
free(filename);
> +free(key);
> +
> +return value;
> +}
> +#endif
just my 2 cents
re,
wh
> +
> static int drmParseSubsystemType(int maj, int min)
> {
> #ifdef __linux__
> @@ -2906,6 +2950,9 @@ static int drmParseSubsystemType(int maj, int min)
> if (strncmp(name, "/pci", 4) == 0)
> return DRM_BUS_PCI;
>
> +if (strncmp(name, "/usb", 4) == 0)
> +return DRM_BUS_USB;
> +
> return -EINVAL;
> #elif defined(__OpenBSD__)
> return DRM_BUS_PCI;
> @@ -2992,6 +3039,10 @@ static int drmCompareBusInfo(drmDevicePtr a,
> drmDevicePtr b)
> switch (a->bustype) {
> case DRM_BUS_PCI:
> return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo));
> +
> +case DRM_BUS_USB:
> +return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
> +
> default:
> break;
> }
> @@ -3235,6 +3286,113 @@ free_device:
> return ret;
> }
>
> +static int drmParseUsbBusInfo(int maj, int min, drmUsbBusInfoPtr info)
> +{
> +#ifdef __linux__
> +char path[PATH_MAX + 1], *value;
> +unsigned int bus, dev;
> +int ret;
> +
> +snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> +
> +value = sysfs_uevent_get(path, "BUSNUM");
> +if (!value)
> +return -ENOENT;
> +
> +ret = sscanf(value, "%03u", &bus);
> +free(value);
> +
> +if (ret <= 0)
> +return -errno;
> +
> +value = sysfs_uevent_get(path, "DEVNUM");
> +if (!value)
> +return -ENOENT;
> +
> +ret = sscanf(value, "%03u", &dev);
> +free(value);
> +
> +if (ret <= 0)
> +return -errno;
> +
> +info->bus = bus;
> +info->dev = dev;
> +
> +return 0;
> +#else
> +#warning "Missing implementation of drmParseUsbBusInfo"
> +return -EINVAL;
> +#endif
> +}
> +
> +static int drmParseUsbDeviceInfo(int maj, int min, drmUsbDeviceInfoPtr info)
> +{
> +#ifdef __linux__
> +char path[PATH_MAX + 1], *value;
> +unsigned int vendor, product;
> +int ret;
> +
> +snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> +
> +value = sysfs_uevent_get(path, "PRODUCT");
> +if (!value)
> +return -ENOENT;
> +
> +ret = sscanf(value, "%x/%x", &vendor, &product);
> +free(value);
> +
> +if (ret <= 0)
> +return -errno;
> +
> +info->vendor = vendor;
> +info->product = product;
> +
> +return 0;
> +#else
> +#warning "Missing implementation of drmParseUsbDeviceInfo"
> +return -EINVAL;
> +#endif
> +}
> +
> +static int drmProcessUsbDevice(drmDevicePtr *device, const char *node,
> + int node_type, int maj, int min,
> + bool fetch_deviceinfo, uint32_t flags)
> +{
> +drmDevicePtr dev;
> +char *ptr;
> +int ret;
> +
> +dev = drmDeviceAlloc(node_type, node, sizeof(drmUsbBusInfo),
> + sizeof(drmUsbDeviceInfo), &ptr);
> +if (!dev)
> +return -ENOMEM;
> +
> +dev->bustype = DRM_BUS_USB;
> +
> +dev->businfo.usb = (drmUsbBusInfoPtr)ptr;
> +
> +ret =
Re: [PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support
Am 18.01.2017 10:02, schrieb Thierry Reding:
> From: Thierry Reding
>
> ARM SoCs usually have their DRM/KMS devices on the platform bus, so add
> support for that to enable these devices to be used with the drmDevice
> infrastructure.
>
> NVIDIA Tegra SoCs have an additional level in the hierarchy and DRM/KMS
> devices can also be on the host1x bus. This is mostly equivalent to the
> platform bus.
>
> v3:
> - guard Linux-specific sysfs parsing code with #ifdef __linux__
>
> v2:
> - be careful not to overflow the full name
> - read compatible strings into device info
>
> Signed-off-by: Thierry Reding
> ---
> xf86drm.c | 300
> ++
> xf86drm.h | 30 ++-
> 2 files changed, 328 insertions(+), 2 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index d83674e638c4..3c6ec1cc9e00 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2953,6 +2953,12 @@ static int drmParseSubsystemType(int maj, int min)
> if (strncmp(name, "/usb", 4) == 0)
> return DRM_BUS_USB;
>
> +if (strncmp(name, "/platform", 9) == 0)
> +return DRM_BUS_PLATFORM;
> +
> +if (strncmp(name, "/host1x", 7) == 0)
> +return DRM_BUS_HOST1X;
> +
> return -EINVAL;
> #elif defined(__OpenBSD__)
> return DRM_BUS_PCI;
> @@ -3043,6 +3049,12 @@ static int drmCompareBusInfo(drmDevicePtr a,
> drmDevicePtr b)
> case DRM_BUS_USB:
> return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
>
> +case DRM_BUS_PLATFORM:
> +return memcmp(a->businfo.platform, b->businfo.platform,
> sizeof(drmPlatformBusInfo));
> +
> +case DRM_BUS_HOST1X:
> +return memcmp(a->businfo.host1x, b->businfo.host1x,
> sizeof(drmHost1xBusInfo));
> +
> default:
> break;
> }
> @@ -3187,11 +3199,55 @@ static int drmParsePciDeviceInfo(int maj, int min,
> #endif
> }
>
> +static void drmFreePlatformDevice(drmDevicePtr device)
> +{
> +if (device->deviceinfo.platform) {
to save 2 indent level and improve readability :
if (! device->deviceinfo.platform ||
! device->deviceinfo.platform->compatible)
return;
> +if (device->deviceinfo.platform->compatible) {
> +char **compatible = device->deviceinfo.platform->compatible;
> +
> +while (*compatible) {
> +free(*compatible);
> +compatible++;
> +}
> +
> +free(device->deviceinfo.platform->compatible);
> +}
> +}
> +}
> +
> +static void drmFreeHost1xDevice(drmDevicePtr device)
> +{
> +if (device->deviceinfo.host1x) {
> +if (device->deviceinfo.host1x->compatible) {
> +char **compatible = device->deviceinfo.host1x->compatible;
> +
> +while (*compatible) {
> +free(*compatible);
> +compatible++;
> +}
> +
> +free(device->deviceinfo.host1x->compatible);
> +}
> +}
> +}
> +
> void drmFreeDevice(drmDevicePtr *device)
> {
> if (device == NULL)
> return;
>
> +if (*device) {
> +switch ((*device)->bustype) {
> +case DRM_BUS_PLATFORM:
> +drmFreePlatformDevice(*device);
> +break;
> +
> +case DRM_BUS_HOST1X:
> +drmFreeHost1xDevice(*device);
> +break;
> +}
> +}
> +
> free(*device);
> *device = NULL;
> }
> @@ -3393,6 +3449,220 @@ free_device:
> return ret;
> }
>
> +static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr
> info)
> +{
> +#ifdef __linux__
> +char path[PATH_MAX + 1], *name;
> +
> +snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> +
> +name = sysfs_uevent_get(path, "OF_FULLNAME");
> +if (!name)
> +return -ENOENT;
> +
> +strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
> +info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
> +free(name);
> +
> +return 0;
> +#else
> +#warning "Missing implementation of drmParsePlatformBusInfo"
> +return -EINVAL;
> +#endif
> +}
> +
> +static int drmParsePlatformDeviceInfo(int maj, int min,
> + drmPlatformDeviceInfoPtr info)
> +{
> +#ifdef __linux__
> +char path[PATH_MAX + 1], *value;
> +unsigned int count, i;
> +int err;
> +
> +snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
> +
> +value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
> +if (!value)
> +return -ENOENT;
> +
> +sscanf(value, "%u", &count);
> +free(value);
> +
> +info->compatible = calloc(count + 1, sizeof(*info->compatible));
> +if (!info->compatible)
> +return -ENOMEM;
> +
> +for (i = 0; i < count; i++) {
> +value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
> +if (!value) {
> +err = -ENOENT;
> +goto free;
> +}
> +
> +info->compatible[i] = value;
> +}
> +
> +
Re: [PATCH libdrm v3 3/5] xf86drm: Add platform and host1x bus support
Am 19.01.2017 11:25, schrieb Thierry Reding:
> Adding back dri-de...@lists.freedesktop.org
>
> On Wed, Jan 18, 2017 at 04:00:20PM +0100, walter harms wrote:
>> Am 18.01.2017 10:02, schrieb Thierry Reding:
> [...]
>>> diff --git a/xf86drm.c b/xf86drm.c
> [...]
>>> @@ -3187,11 +3199,55 @@ static int drmParsePciDeviceInfo(int maj, int min,
>>> #endif
>>> }
>>>
>>> +static void drmFreePlatformDevice(drmDevicePtr device)
>>> +{
>>> +if (device->deviceinfo.platform) {
>>
>> to save 2 indent level and improve readability :
>>
>> if (! device->deviceinfo.platform ||
>> ! device->deviceinfo.platform->compatible)
>> return;
>>
>>> +if (device->deviceinfo.platform->compatible) {
>>> +char **compatible = device->deviceinfo.platform->compatible;
>>> +
>>> +while (*compatible) {
>>> +free(*compatible);
>>> +compatible++;
>>> +}
>>> +
>>> +free(device->deviceinfo.platform->compatible);
>>> +}
>>> +}
>
> I chose the above style because I thought readability wasn't impacted
> and it has the advantage of making the code more easily extensible
> should we ever need to add code to it.
i prefer otherwise, NTL its your patch if you are happy its ok for me
>
>>> +static int drmProcessHost1xDevice(drmDevicePtr *device,
>>> + const char *node, int node_type,
>>> + int maj, int min, bool fetch_deviceinfo,
>>> + uint32_t flags)
>>> +{
>>> +drmDevicePtr dev;
>>> +char *ptr;
>>> +int ret;
>>> +
>>> +dev = drmDeviceAlloc(node_type, node, sizeof(drmHost1xBusInfo),
>>> + sizeof(drmHost1xDeviceInfo), &ptr);
>>> +if (!dev)
>>> +return -ENOMEM;
>>> +
>>> +dev->bustype = DRM_BUS_HOST1X;
>>> +
>>> +dev->businfo.host1x = (drmHost1xBusInfoPtr)ptr;
>>> +
>>> +ret = drmParseHost1xBusInfo(maj, min, dev->businfo.host1x);
>>> +if (ret < 0)
>>> +goto free_device;
>>> +
>>> +if (fetch_deviceinfo) {
>>> +ptr += sizeof(drmHost1xBusInfo);
>>> +dev->deviceinfo.host1x = (drmHost1xDeviceInfoPtr)ptr;
>>> +
>>> +ret = drmParseHost1xDeviceInfo(maj, min, dev->deviceinfo.host1x);
>>> +if (ret < 0)
>>> +goto free_device;
>>> +}
>>> +
>>> +*device = dev;
>>> +
>>
>> do you assume that fetch_deviceinfo may change dev ?
>
> No, why?
You do *device = dev; after the if block that leaves the impression (for me)
that dev may change somewhere. therefor i would suggest moving this upwards.
re,
wh
>
> Thierry
>
>
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libdrm v3 2/5] xf86drm: Add USB support
Am 19.01.2017 11:20, schrieb Thierry Reding:
> Adding back dri-de...@lists.freedesktop.org
>
> On Wed, Jan 18, 2017 at 12:21:23PM +0100, walter harms wrote:
>>
>>
>> Am 18.01.2017 10:02, schrieb Thierry Reding:
>>> Allow DRM/KMS devices hosted on USB to be detected by the drmDevice
>>> infrastructure.
>>>
>>> v3:
>>> - guard Linux-specific sysfs parsing code with #ifdef __linux__
>>>
>>> v2:
>>> - make sysfs_uevent_get() more flexible using a format string
>>>
>>> Signed-off-by: Thierry Reding
>>> ---
>>> xf86drm.c | 175
>>> ++
>>> xf86drm.h | 13 +
>>> 2 files changed, 188 insertions(+)
>>>
>>> diff --git a/xf86drm.c b/xf86drm.c
>>> index 7766bfe937db..d83674e638c4 100644
>>> --- a/xf86drm.c
>>> +++ b/xf86drm.c
>>> @@ -2886,6 +2886,50 @@ char *drmGetRenderDeviceNameFromFd(int fd)
>>> return drmGetMinorNameForFD(fd, DRM_NODE_RENDER);
>>> }
>>>
>>> +#ifdef __linux__
>>> +static char * DRM_PRINTFLIKE(2, 3)
>>> +sysfs_uevent_get(const char *path, const char *fmt, ...)
>>> +{
>>> +char filename[PATH_MAX + 1], *key, *line = NULL, *value = NULL;
>>char *filename=NULL, *key, *line = NULL, *value = NULL;
>>> +size_t size = 0, len;
>>> +ssize_t num;
>>> +va_list ap;
>>> +FILE *fp;
>>> +
>>> +va_start(ap, fmt);
>>> +num = vasprintf(&key, fmt, ap);
>>> +va_end(ap);
>>> +len = num;
>>> +
>>> +snprintf(filename, sizeof(filename), "%s/uevent", path);
>>
>> since asprintf() is available you could use:
>>
>> asprintf(&filename,"%s/uevent", path);
>>
>> same could be done for path below.
>
> I had thought about that, but a stack-allocated string seemed
> advantageous for three reasons:
>
> - asprintf() is a GNU extension. That's not much of an issue
> because this is already protected by #ifdef __linux__ and
> pretty much all C libraries I know support asprintf() and
> friends.
>
> - PATH_MAX is the maximum length of a filename, so there's no
> need to dynamically allocate, since it should nicely fit into
> the stack pretty much everywhere (I think the largest value I
> have ever seen for PATH_MAX is 4096 on recent Linux systems).
>
> - Most of the other code in xf86drm.c already uses PATH_MAX, so
> the code remains consistent.
>
> Given the added complexity of asprintf() and the need to free the memory
> the advantages of the stack-allocated string seemed to outweigh.
>
> Thierry
Granted,
I like to use asprintf every time %s is used to stop malicious users in the
tracks
(do not aks me how this could be done with our function). Freeing the allocated
buf
is no practical problem. NTL i have no problem if you are happy with the patch,
making
an informed decision is ok with me.
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: libx11: Issues with Data32/_XData32
Am 24.01.2017 09:18, schrieb Julien Cristau: > On Tue, Jan 24, 2017 at 01:12:23 +, James Clarke wrote: > >> Hi, >> I've been debugging an issue in gtk2-perl causing it to SIGBUS on >> sparc64, and traced it back to what seems to be dodgy code inside >> libx11. One of the tests calls gdk_window_set_opacity, which calls >> XChangeProperty with a pointer to a guint32, cast to char*, with the >> length set to 32 bits as expected. However, this data pointer then gets >> cast to a (long *) on line 83 of ChProp.c when calling Data32. Indeed, >> the definition of Data32 specifies that data is a (long *) on sparc64, >> since LONG64 is defined. This feels very wrong, but in and of itself is >> not too bad (I believe it violates strict aliasing). >> > As discussed on irc this sounds like a gtk bug, fixed by > https://git.gnome.org/browse/gtk+/commit/?id=0e1a424829937abc27780da97a8bf60e81233d6c > for gtk 3. > I have read the page and found that sentence but i believe that it is realy hard to find if you do not know where to look. I would suggest to add a small example to clarify the problem. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH edid-decode v2 2/2] Report broken CEA and DisplayID checksums.
look good for me.
Reviewed-by: wha...@bfs.de
Am 06.02.2017 14:51, schrieb Mark Ferry:
> ---
> edid-decode.c | 21 ++---
> 1 file changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index 15660dc..95e9c96 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -50,6 +50,8 @@ static int name_descriptor_terminated = 0;
> static int has_range_descriptor = 0;
> static int has_preferred_timing = 0;
> static int has_valid_checksum = 1;
> +static int has_valid_cea_checksum = 1;
> +static int has_valid_displayid_checksum = 1;
> static int has_valid_cvt = 1;
> static int has_valid_dummy_block = 1;
> static int has_valid_week = 0;
> @@ -1289,7 +1291,7 @@ parse_cea(unsigned char *x)
> detailed_block(detailed, 1);
> } while (0);
>
> -do_checksum(x, EDID_PAGE_SIZE);
> +has_valid_cea_checksum = do_checksum(x, EDID_PAGE_SIZE);
>
> return ret;
> }
> @@ -1380,7 +1382,11 @@ parse_displayid(unsigned char *x)
> int i;
> printf("Length %d, version %d, extension count %d\n", length, version,
> ext_count);
>
> -do_checksum(x+1, length + 5);
> +/* DisplayID length field is number of following bytes
> + * but checksum is calculated over the entire structure
> + * (excluding DisplayID-in-EDID magic byte)
> + */
> +has_valid_displayid_checksum = do_checksum(x+1, length + 5);
>
> int offset = 5;
> while (length > 0) {
> @@ -2048,7 +2054,7 @@ int main(int argc, char **argv)
> has_valid_extension_count = 1;
> }
>
> -do_checksum(edid, EDID_PAGE_SIZE);
> +has_valid_checksum = do_checksum(edid, EDID_PAGE_SIZE);
>
> x = edid;
> for (edid_lines /= 8; edid_lines > 1; edid_lines--) {
> @@ -2142,6 +2148,15 @@ int main(int argc, char **argv)
> printf("\tEDID 1.4 block does not set max dotclock\n");
> }
>
> +if (!has_valid_cea_checksum) {
> +printf("CEA extension block does not conform\n");
> +printf("\tBlock has broken checksum\n");
> +}
> +if (!has_valid_displayid_checksum) {
> +printf("DisplayID extension block does not conform\n");
> +printf("\tBlock has broken checksum\n");
> +}
> +
> if (warning_excessive_dotclock_correction)
> printf("Warning: CVT block corrects dotclock by more than 9.75MHz\n");
> if (warning_zero_preferred_refresh)
>
>
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH edid-decode v2 1/2] Make do_checksum reusable for DisplayID checksums.
look good for me.
Reviewed-by: wha...@bfs.de
Am 06.02.2017 14:51, schrieb Mark Ferry:
> DisplayID, unlike EDID and CEA, is a variable length structure.
> Allow for reuse of do_checksum by adding a length parameter.
>
> Return a boolean to allow the caller to record failure.
> ---
> edid-decode.c | 45 -
> 1 file changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index c18697f..15660dc 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -34,6 +34,10 @@
>
> #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
>
> +enum {
> +EDID_PAGE_SIZE = 128u
> +};
> +
> static int claims_one_point_oh = 0;
> static int claims_one_point_two = 0;
> static int claims_one_point_three = 0;
> @@ -208,7 +212,7 @@ detailed_cvt_descriptor(unsigned char *x, int first)
> static char *
> extract_string(unsigned char *x, int *valid_termination, int len)
> {
> -static char ret[128];
> +static char ret[EDID_PAGE_SIZE];
> int i, seen_newline = 0;
>
> memset(ret, 0, sizeof(ret));
> @@ -560,21 +564,25 @@ detailed_block(unsigned char *x, int in_extension)
> return 1;
> }
>
> -static void
> -do_checksum(unsigned char *x)
> +static int
> +do_checksum(unsigned char *x, size_t len)
> {
> -printf("Checksum: 0x%hx", x[0x7f]);
> -{
> - unsigned char sum = 0;
> - int i;
> - for (i = 0; i < 128; i++)
> - sum += x[i];
> - if (sum) {
> - printf(" (should be 0x%hx)", (unsigned char)(x[0x7f] - sum));
> - has_valid_checksum = 0;
> - } else printf(" (valid)");
> +unsigned char check = x[len - 1];
> +unsigned char sum = 0;
> +int i;
> +
> +printf("Checksum: 0x%hx", check);
> +
> +for (i = 0; i < len-1; i++)
> +sum += x[i];
> +
> +if ((unsigned char)(check + sum) != 0) {
> +printf(" (should be 0x%hx)\n", -sum & 0xff);
> +return 0;
> }
> -printf("\n");
> +
> +printf(" (valid)\n");
> +return 1;
> }
>
> /* CEA extension */
> @@ -1281,7 +1289,7 @@ parse_cea(unsigned char *x)
> detailed_block(detailed, 1);
> } while (0);
>
> -do_checksum(x);
> +do_checksum(x, EDID_PAGE_SIZE);
>
> return ret;
> }
> @@ -1371,6 +1379,9 @@ parse_displayid(unsigned char *x)
> int ext_count = x[4];
> int i;
> printf("Length %d, version %d, extension count %d\n", length, version,
> ext_count);
> +
> +do_checksum(x+1, length + 5);
> +
> int offset = 5;
> while (length > 0) {
> int tag = x[offset];
> @@ -2037,11 +2048,11 @@ int main(int argc, char **argv)
> has_valid_extension_count = 1;
> }
>
> -do_checksum(edid);
> +do_checksum(edid, EDID_PAGE_SIZE);
>
> x = edid;
> for (edid_lines /= 8; edid_lines > 1; edid_lines--) {
> - x += 128;
> + x += EDID_PAGE_SIZE;
> nonconformant_extension += parse_extension(x);
> }
>
>
>
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xim] Send screen number
Am 17.02.2017 04:33, schrieb Takao Fujiwara: > Hi, > I'm a maintainer of IBus which is an input method framework. > Xorg ZaphodHeads environment provides the multiple screens. > I think currently XIM protocol does not send the $DISPLAY information in > the client applications to the XIM server and IBus cannot know the > screen number of the client applications. > > The following patch can send the client DISPLAY information to the XIM > server: > > https://github.com/fujiwarat/libX11/commits/xim-screen-number > > Could you evaluate the patch? > Fujiwara Hi Fujiwara, thx for your efforts. when you want your patch into the mainline please have the inline your mail and add a Signed-off-by: see as example: [PATCH xserver] sdksyms: Tighten up the symbols we add to the magic table General info: https://www.x.org/wiki/Development/Documentation/SubmittingPatches/ hope that helps, re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: (timeout in ms vs. XSyncValueSubtract) Frozen client, found cause, need advise for fix
Am 20.02.2017 10:55, schrieb Daniel Martin:
> Hi everyone,
>
> I've got a Qt5 client, which receives mouse input only and freezes
> soon and often. It gets stuck waiting for a reply from the server.
> Pressing a none-modifier key releases the lock and the client
> continues as normal until the next freeze.
>
> I've traced the bug down to a regression implicitly introduced by:
> e6636b4 os: Compute timeout in milliseconds instead of struct timeval
> Which states "simplifies the code without affecting functionality at all". ;-)
>
> What happens is:
> - ServertimeBlockHandler() forwards a _big_ unsigned timeout to
> AdjustWaitForDelay()
> - AdjustWaitForDelay() takes this _big_ unsigned as int and it becomes
> a negative value
> - the negative value ends up as timeout in ospoll_wait() as parameter
> to epoll_wait()
>> epoll_wait() blocks due to the negative timeout
>
> XSyncValueSubtract() doesn't handle unsigned wraps. That's why we get
> a big unsigned from ServertimeBlockHandler(). Just imagine two
> XSyncValues:
> a = {.hi=1, .lo=1} and b = {.hi=0, lo=.2}.
> The result of XSyncValueSubtract(a - b) is
> presult = {.hi=0, .lo=4294967295},
> where I would expect
>presult = {.hi=0, .lo=9}.
>
> If .lo=4294967295 is wrong and .lo=9 is what we want, could anyone
> provide a fix, please? I'm to dumb for the (binary?) arithmetic atm.
> (And if it is wrong, did I just uncovered an ancient bug?)
>
XSyncValueSubtract is doing as expected,
XSyncValue is the simulation of 64bitvalues on 32bit.
see this in hex:
10001
-2
0 = 4294967295 in .lo
> To paper over the (mis)behaviour of XSyncValueSubtract() I would
> suggest to make sure that AdjustWaitForDelay() doesn't return negative
> values. We can't check for negatives in ospoll_wait() as the input
> thread calls it with -1.
no idea, i suspect a mix of unit somewhere seconds vs mseconds or so.
re,
wh
>
> Other thoughts: Why does the cursor move, but neither the movement or
> clicks, nor pressing a modifier unlock the client?
>
>
> Cheers,
> Daniel Martin
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: (timeout in ms vs. XSyncValueSubtract) Frozen client, found cause, need advise for fix
Am 21.02.2017 20:28, schrieb Mihail Konev:
> On Tue, Feb 21, 2017 at 05:48:19PM +0100, walter harms wrote:
>>>
>>
>> XSyncValueSubtract is doing as expected,
>> XSyncValue is the simulation of 64bitvalues on 32bit.
>> see this in hex:
>> 10001
>> -2
>> 0 = 4294967295 in .lo
>>
>
> Kind of - it errorneously returns
>
> 1 0001
> -0 0002
> 1
>
> Test source attached.
>
> I.e.
> XSyncValueSubtract should handle the .lo becoming <0,
> and it checks only for .lo_after_subtract < .lo,
> which would only be aplicable to uint.
>
> E.g. there is a typo in XSyncValue.lo definition - it should be uint
> instead.
> Also in ServertimeBlockHandler, "timeout" is unsigned,
> while it is get from signed .lo and used as signed arg to
> AdjustWaitForTimeout.
yep, i noticed that "int t = (a).lo;" and was confused. But the experiments
showed
is seems to work.
#define _XSyncValueSubtract(presult,a,b,poverflow) {\
int t = (a).lo;\
Bool signa = XSyncValueIsNegative(a);\
Bool signb = XSyncValueIsNegative(b);\
((presult)->lo = (a).lo - (b).lo);\
((presult)->hi = (a).hi - (b).hi);\
if (t>(presult)->lo) (presult)->hi--;\
*poverflow = ((signa == signb) && !(signa == XSyncValueIsNegative(*presu
lt)));\
}
my test code:
#include
#include
#include
// gcc xsync.c -lXext
int main()
{
XSyncValue a,b,c;
XSyncValue result;
Bool ov;
XSyncIntToValue(&a, 100);
XSyncIntToValue(&b, 200);
XSyncValueSubtract(&result, b, a, &ov);
printf("result.hi = %d, result.lo = %d, overflow = %d\n", result.hi,
result.lo, ov);
}
I did not look at the asm level but the interessting question is:
can that be replace by int64_t ? is it done somewhere else ?
re,
wh
>
> Perhaps also:
> AdjustWaitForTimeout should not be used if overflow != 0
> Because something is wrong then as (pnext_time < Now) ?
>
> Mihail
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [RFC xserver] os: log a bug whenever WriteToClient is called from the input thread
Am 23.02.2017 01:22, schrieb Peter Hutterer:
> The input thread should generate events, not send them. Make it easier to
> find the instances where it's doing so.
>
> Signed-off-by: Peter Hutterer
> ---
> include/input.h | 1 +
> os/inputthread.c | 7 +++
> os/io.c | 6 ++
> 3 files changed, 14 insertions(+)
>
> diff --git a/include/input.h b/include/input.h
> index bb58b22..6c9e45d 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -722,6 +722,7 @@ extern _X_HIDDEN void input_constrain_cursor(DeviceIntPtr
> pDev, ScreenPtr screen
> extern _X_EXPORT void input_lock(void);
> extern _X_EXPORT void input_unlock(void);
> extern _X_EXPORT void input_force_unlock(void);
> +extern _X_EXPORT int in_input_thread(void);
>
> extern void InputThreadPreInit(void);
> extern void InputThreadInit(void);
> diff --git a/os/inputthread.c b/os/inputthread.c
> index 4400fba..933bc1c 100644
> --- a/os/inputthread.c
> +++ b/os/inputthread.c
> @@ -90,6 +90,12 @@ static pthread_mutex_t input_mutex;
> static Bool input_mutex_initialized;
> #endif
>
> +int
> +in_input_thread(void)
> +{
> +return pthread_equal(pthread_self(), inputThreadInfo->thread);
> +}
> +
> void
> input_lock(void)
> {
> @@ -531,6 +537,7 @@ void input_force_unlock(void) {}
> void InputThreadPreInit(void) {}
> void InputThreadInit(void) {}
> void InputThreadFini(void) {}
> +int in_input_thread(void) { return 0; }
>
> int InputThreadRegisterDev(int fd,
> NotifyFdProcPtr readInputProc,
> diff --git a/os/io.c b/os/io.c
> index be85226..5494b30 100644
> --- a/os/io.c
> +++ b/os/io.c
> @@ -643,6 +643,9 @@ SetCriticalOutputPending(void)
> *this routine as int.
> */
>
> +extern inline int
> +in_input_thread(void);
> +
maybe i am old fashion but this should be done with input.h, should it ?
re,
wh
> int
> WriteToClient(ClientPtr who, int count, const void *__buf)
> {
> @@ -651,6 +654,9 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
> int padBytes;
> const char *buf = __buf;
>
> +BUG_RETURN_VAL_MSG(in_input_thread(), 0,
> + " %s called from input thread *\n",
> __func__);
> +
> #ifdef DEBUG_COMMUNICATION
> Bool multicount = FALSE;
> #endif
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [RFC xserver] os: log a bug whenever WriteToClient is called from the input thread
Am 23.02.2017 17:16, schrieb Olivier Fourdan: > Hi, > >>> +extern inline int >>> +in_input_thread(void); >>> + >> >> maybe i am old fashion but this should be done with input.h, should it ? > > No, I don't think this is possible (in the middle of a stable release) nor > even suitable, it's not a new API intended for drivers to use, it's purely > internal to the Xserver to tell if it's safe to send data to clients (i.e. if > we are on the main thread and not in the input thread) > > Cheers, > Olivier. What is about a comment like /* INTERNAL USE ONLY */ Otherwise someone else will notice the prototype in input.h re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xf86-input-libinput 1/3] Add streq() macro, replace strcmp instances with it
Am 24.02.2017 15:13, schrieb Eric Engestrom:
> On Friday, 2017-02-24 13:10:17 +1000, Peter Hutterer wrote:
>> And why isn't this a thing in glibc yet
>
> Indeed :(
> So many bugs caused by someone assuming `if (strcmp(a, b))` means a==b...
>
>>
>> Signed-off-by: Peter Hutterer
>> ---
>> src/xf86libinput.c | 21 -
>> 1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
>> index 703d872..c1214b7 100644
>> --- a/src/xf86libinput.c
>> +++ b/src/xf86libinput.c
>> @@ -65,6 +65,9 @@
>> #define TOUCH_MAX_SLOTS 15
>> #define XORG_KEYCODE_OFFSET 8
>>
>> +#define streq(a, b) (strcmp(a, b) == 0)
>> +#define strneq(a, b, n) (strncmp(a, b, n) == 0)
>
> strneq() reads to me as "string not equal"...
> streqn() might be a better name?
>
perhaps turn around:
eqstrn
eqstr
re,
wh
> And a bit of a nitpick, but could you move its introduction to the patch
> that starts using it (ie. 3/3)?
>
> Otherwise, this patch is:
> Reviewed-by: Eric Engestrom
>
> (the other patches require more knowledge than I have)
>
>> +
>> /*
>> libinput does not provide axis information for absolute devices, instead
>> it scales into the screen dimensions provided. So we set up the axes with
>> @@ -259,7 +262,7 @@ xf86libinput_is_subdevice(InputInfoPtr pInfo)
>> BOOL is_subdevice;
>>
>> source = xf86SetStrOption(pInfo->options, "_source", "");
>> -is_subdevice = strcmp(source, "_driver/libinput") == 0;
>> +is_subdevice = streq(source, "_driver/libinput");
>> free(source);
>>
>> return is_subdevice;
>> @@ -1213,7 +1216,7 @@ is_libinput_device(InputInfoPtr pInfo)
>> BOOL rc;
>>
>> driver = xf86CheckStrOption(pInfo->options, "driver", "");
>> -rc = strcmp(driver, "libinput") == 0;
>> +rc = streq(driver, "libinput");
>> free(driver);
>>
>> return rc;
>> @@ -2187,7 +2190,7 @@ open_restricted(const char *path, int flags, void
>> *data)
>> nt_list_for_each_entry(pInfo, xf86FirstLocalDevice(), next) {
>> char *device = xf86CheckStrOption(pInfo->options, "Device",
>> NULL);
>>
>> -if (device != NULL && strcmp(path, device) == 0) {
>> +if (device != NULL && streq(path, device)) {
>> free(device);
>> break;
>> }
>> @@ -2353,9 +2356,9 @@ xf86libinput_parse_tap_buttonmap_option(InputInfoPtr
>> pInfo,
>> "TappingButtonMap",
>> NULL);
>> if (str) {
>> -if (strcmp(str, "lmr") == 0)
>> +if (streq(str, "lmr"))
>> map = LIBINPUT_CONFIG_TAP_MAP_LMR;
>> -else if (strcmp(str, "lrm") == 0)
>> +else if (streq(str, "lrm"))
>> map = LIBINPUT_CONFIG_TAP_MAP_LRM;
>> else
>> xf86IDrvMsg(pInfo, X_ERROR,
>> @@ -2468,11 +2471,11 @@ xf86libinput_parse_sendevents_option(InputInfoPtr
>> pInfo,
>> "SendEventsMode",
>> NULL);
>> if (modestr) {
>> -if (strcmp(modestr, "enabled") == 0)
>> +if (streq(modestr, "enabled"))
>> mode = LIBINPUT_CONFIG_SEND_EVENTS_ENABLED;
>> -else if (strcmp(modestr, "disabled") == 0)
>> +else if (streq(modestr, "disabled"))
>> mode = LIBINPUT_CONFIG_SEND_EVENTS_DISABLED;
>> -else if (strcmp(modestr, "disabled-on-external-mouse") == 0)
>> +else if (streq(modestr, "disabled-on-external-mouse"))
>> mode =
>> LIBINPUT_CONFIG_SEND_EVENTS_DISABLED_ON_EXTERNAL_MOUSE;
>> else
>> xf86IDrvMsg(pInfo, X_ERROR,
>> @@ -2866,7 +2869,7 @@ xf86libinput_parse_tablet_area_option(InputInfoPtr
>> pInfo,
>> str = xf86SetStrOption(pInfo->options,
>> "TabletToolAreaRatio",
>> NULL);
>> -if (!str || strcmp(str, "default") == 0)
>> +if (!str || streq(str, "default"))
>> goto out;
>>
>> rc = sscanf(str, "%d:%d", &area.x, &area.y);
>> --
>> 2.9.3
>>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] dmx: Fix null pointer dereference
Am 12.03.2017 14:21, schrieb Tobias Stoeckmann:
> A null pointer dereference can occur in dmxSync, because TimerForce
> does not handle a null pointer.
>
> dmxSyncTimer is set to NULL a few lines above on a certain condition,
> which happened on my machine. The explicit NULL check allowed me to
> start Xdmx again without a segmentation fault.
> ---
> hw/dmx/dmxsync.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/dmx/dmxsync.c b/hw/dmx/dmxsync.c
> index 1bc242343..b55c9ddf3 100644
> --- a/hw/dmx/dmxsync.c
> +++ b/hw/dmx/dmxsync.c
> @@ -182,7 +182,7 @@ dmxSync(DMXScreenInfo * dmxScreen, Bool now)
>
> /* Do sync or set time for later */
> if (now || !dmxScreen) {
> -if (!TimerForce(dmxSyncTimer))
> +if (dmxSyncTimer == NULL || !TimerForce(dmxSyncTimer))
> dmxSyncCallback(NULL, 0, NULL);
> /* At this point, dmxSyncPending == 0 because
> * dmxSyncCallback must have been called. */
why not patch TimerForce() and solve the problem for once and any one ?
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH rendercheck 0/5] Convert to meson.
Am 24.03.2017 22:13, schrieb Mark Kettenis: >> From: Eric Anholt >> Date: Fri, 24 Mar 2017 13:17:45 -0700 >> >> Having bitten off a bit more than I can chew in 3 days with the X >> Server (hw/xfree86/sdksyms.c is the worst), I decided to take a quick >> pass at converting a project that's my own fault. > > Seems I missed some discussion somewhere... > > While I understand your frustrations with autoconf, moving to build > infrastructure that relies on tools like Python and Ninja would have > serious consequences for the way we integrate X into OpenBSD. We > build the entire base OS, which includes X, with tools that are part > of the base OS. That would become pretty much impossible if various X > projects go this way. > > This is the first I've heard about Meson. That in itself doesn't fill > me with confidence. But the Meson documentation doesn't really > mentioned how you'd do configure-style checks like checking whether a > particular header file is present or whether a library defines a > certain function. I suspect the answer is that people using Meson > don't do such tests and don't feel the need to do such tests because > they just check whether you're running on Windows, MacOS X or Linux > and make assumptions that things are present based on just that. Your > new meson build system doesn't check whether the err.h file exists or > whether the C compiler supports the -Wshadow option for example. It's > not really surprising that Meson is much faster if you cut out all > those checks. Unfortunately this approach tends to yield rather > unportable software. > > You can probably cut quite a bit of time from the autoconf configure > script run time by removing silly checks like checking for standard C > headers and such. I do not like the idea either. Autotools are far from perfekt but do there job and you do not need more that the macros provided what you can get from freedektop git. The last time i tried a phyton installer (a few weeks ago) i found my self downloading different moduls for all over the net and in the end it did not work at all. At last i did the install by hand. This was not the first time, and i fear not the last time. Since imake several attempts have been done and to be fair i found them not convincing. things can get much more worse than autotools. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/3] xfree86: Silence always true condition warning.
Am 27.03.2017 14:03, schrieb m...@suse.com: > From: Michal Srb > > xf86pciBus.c:1464:21: warning: comparison of constant 256 with expression of > type 'uint8_t' (aka 'unsigned char') is always true > [-Wtautological-constant-out-of-range-compare] > if (pVideo->bus < 256) > > The code used to be in xf86FormatPciBusNumber and compared parameter which > was int, but since b967bf2a it was inlined now it works with uint8_t. > --- > hw/xfree86/common/xf86pciBus.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c > index 9adfee5..ae03b75 100644 > --- a/hw/xfree86/common/xf86pciBus.c > +++ b/hw/xfree86/common/xf86pciBus.c > @@ -1461,11 +1461,7 @@ xf86PciConfigureNewDev(void *busData, struct > pci_device *pVideo, > > pVideo = (struct pci_device *) busData; > > -if (pVideo->bus < 256) > -snprintf(busnum, sizeof(busnum), "%d", pVideo->bus); > -else > -snprintf(busnum, sizeof(busnum), "%d@%d", > - pVideo->bus & 0x00ff, pVideo->bus >> 8); > +snprintf(busnum, sizeof(busnum), "%d", pVideo->bus); > > XNFasprintf(&tmp, "PCI:%s:%d:%d", > busnum, pVideo->dev, pVideo->func); is busnum used later ? otherwise you could use the XNFasprintf(). re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 xserver 1/3] xfree86: Silence always true condition warning.
looks nice
Reviewed-by: wha...@bfs.de
Am 27.03.2017 17:36, schrieb Michal Srb:
> xf86pciBus.c:1464:21: warning: comparison of constant 256 with expression of
> type 'uint8_t' (aka 'unsigned char') is always true
> [-Wtautological-constant-out-of-range-compare]
> if (pVideo->bus < 256)
>
> The code used to be in xf86FormatPciBusNumber and compared parameter which
> was int, but since b967bf2a it was inlined now it works with uint8_t.
>
> v2: Merge the snprintf into the following XNFasprintf.
> ---
> hw/xfree86/common/xf86pciBus.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
> index 9adfee5..d589ece 100644
> --- a/hw/xfree86/common/xf86pciBus.c
> +++ b/hw/xfree86/common/xf86pciBus.c
> @@ -1456,19 +1456,12 @@ void
> xf86PciConfigureNewDev(void *busData, struct pci_device *pVideo,
> GDevRec * GDev, int *chipset)
> {
> -char busnum[8];
> char *tmp;
>
> pVideo = (struct pci_device *) busData;
>
> -if (pVideo->bus < 256)
> -snprintf(busnum, sizeof(busnum), "%d", pVideo->bus);
> -else
> -snprintf(busnum, sizeof(busnum), "%d@%d",
> - pVideo->bus & 0x00ff, pVideo->bus >> 8);
> -
> -XNFasprintf(&tmp, "PCI:%s:%d:%d",
> -busnum, pVideo->dev, pVideo->func);
> +XNFasprintf(&tmp, "PCI:%d:%d:%d",
> +pVideo->bus, pVideo->dev, pVideo->func);
> GDev->busID = tmp;
>
> GDev->chipID = pVideo->device_id;
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] dix: Don't leak swapped events
Am 28.03.2017 19:42, schrieb Adam Jackson:
> The original code here is silly. Just allocate an event on the stack and
> swap into that instead of forcing an ever-growing array into the heap.
>
> Signed-off-by: Adam Jackson
> ---
> dix/events.c | 23 +--
> 1 file changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/dix/events.c b/dix/events.c
> index cc26ba5..fd3434f 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -266,9 +266,6 @@ static struct DeviceEventTime {
> */
> #define RootWindow(sprite) sprite->spriteTrace[0]
>
> -static xEvent *swapEvent = NULL;
> -static int swapEventLen = 0;
> -
> void
> NotImplemented(xEvent *from, xEvent *to)
> {
> @@ -5897,7 +5894,6 @@ WriteEventsToClient(ClientPtr pClient, int count,
> xEvent *events)
> #ifdef PANORAMIX
> xEvent eventCopy;
> #endif
> -xEvent *eventTo, *eventFrom;
> int i, eventlength = sizeof(xEvent);
>
> if (!pClient || pClient == serverClient || pClient->clientGone)
> @@ -5972,25 +5968,16 @@ WriteEventsToClient(ClientPtr pClient, int count,
> xEvent *events)
> }
>
> if (pClient->swapped) {
> -if (eventlength > swapEventLen) {
> -swapEventLen = eventlength;
> -swapEvent = realloc(swapEvent, swapEventLen);
> -if (!swapEvent) {
> -FatalError("WriteEventsToClient: Out of memory.\n");
> -return;
> -}
> -}
> -
> for (i = 0; i < count; i++) {
> -eventFrom = &events[i];
> -eventTo = swapEvent;
> +xEvent swapped;
>
> +memset(&swapped, 0, sizeof swapped);
> /* Remember to strip off the leading bit of type in case
> this event was sent with "SendEvent." */
> -(*EventSwapVector[eventFrom->u.u.type & 0177])
> -(eventFrom, eventTo);
> +(*EventSwapVector[events[i].u.u.type & 0177])
> +(&events[i], &swapped);
you really what to keep it octal here ? not just 0xFF ?
re,
wh
>
> -WriteToClient(pClient, eventlength, eventTo);
> +WriteToClient(pClient, eventlength, &swapped);
> }
> }
> else {
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXdmcp] Use getrandom() syscall if available
Am 03.04.2017 14:52, schrieb Benjamin Tissoires: > This allows to fix CVE-2017-2625 on Linux platforms without pulling in > libbsd. > The syscall getrandom is available since kernel v3.17. The code first > tries to use the syscall on a supported kernel. If the syscall fails, > it falls back to the current (vulnerable) code. > We do not implement the glibc getrandom() call given that it's only > available in glibc 2.25, and the #if dance is already messy here. > > Signed-off-by: Benjamin Tissoires > --- > Key.c| 12 > configure.ac | 3 +++ > 2 files changed, 15 insertions(+) > > diff --git a/Key.c b/Key.c > index a09b316..61b07db 100644 > --- a/Key.c > +++ b/Key.c > @@ -36,6 +36,10 @@ in this Software without prior written authorization from > The Open Group. > #include /* for arc4random_buf() */ > #endif > > +#if HAVE_DECL_SYS_GETRANDOM > +#include /* for SYS_getrandom */ > +#endif > + > #ifndef HAVE_ARC4RANDOM_BUF > static void > getbits (long data, unsigned char *dst) > @@ -68,6 +72,14 @@ XdmcpGenerateKey (XdmAuthKeyPtr key) > #ifndef HAVE_ARC4RANDOM_BUF > longlowbits, highbits; > > +#if HAVE_DECL_SYS_GETRANDOM > +int ret; > + > +ret = syscall(SYS_getrandom, key->data, 8, 0); > +if (ret == 8) > + return; > +#endif > + i am not an expert on syscalls but would it help to test for SYS_getrandom directly ? re, wh > srandom ((int)getpid() ^ time((Time_t *)0)); > lowbits = random (); > highbits = random (); > diff --git a/configure.ac b/configure.ac > index 2288502..d0d4d05 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -63,6 +63,9 @@ case $host_os in > ;; > esac > > +# Checks for syscalls > +AC_CHECK_DECLS([SYS_getrandom], [], [], [[#include ]]) > + > # Checks for library functions. > AC_CHECK_LIB([bsd], [arc4random_buf]) > AC_CHECK_FUNCS([srand48 lrand48 arc4random_buf]) ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXdmcp] Use getrandom() syscall if available
Am 03.04.2017 17:30, schrieb Benjamin Tissoires: > On Mon, Apr 3, 2017 at 3:17 PM, walter harms wrote: >> >> >> Am 03.04.2017 14:52, schrieb Benjamin Tissoires: >>> This allows to fix CVE-2017-2625 on Linux platforms without pulling in >>> libbsd. >>> The syscall getrandom is available since kernel v3.17. The code first >>> tries to use the syscall on a supported kernel. If the syscall fails, >>> it falls back to the current (vulnerable) code. >>> We do not implement the glibc getrandom() call given that it's only >>> available in glibc 2.25, and the #if dance is already messy here. >>> >>> Signed-off-by: Benjamin Tissoires >>> --- >>> Key.c| 12 >>> configure.ac | 3 +++ >>> 2 files changed, 15 insertions(+) >>> >>> diff --git a/Key.c b/Key.c >>> index a09b316..61b07db 100644 >>> --- a/Key.c >>> +++ b/Key.c >>> @@ -36,6 +36,10 @@ in this Software without prior written authorization >>> from The Open Group. >>> #include /* for arc4random_buf() */ >>> #endif >>> >>> +#if HAVE_DECL_SYS_GETRANDOM >>> +#include /* for SYS_getrandom */ >>> +#endif >>> + >>> #ifndef HAVE_ARC4RANDOM_BUF >>> static void >>> getbits (long data, unsigned char *dst) >>> @@ -68,6 +72,14 @@ XdmcpGenerateKey (XdmAuthKeyPtr key) >>> #ifndef HAVE_ARC4RANDOM_BUF >>> longlowbits, highbits; >>> >>> +#if HAVE_DECL_SYS_GETRANDOM >>> +int ret; >>> + >>> +ret = syscall(SYS_getrandom, key->data, 8, 0); >>> +if (ret == 8) >>> + return; >>> +#endif >>> + >> >> i am not an expert on syscalls but would it help to test for >> SYS_getrandom directly ? > > I am not sure I fully understand you. Are you suggesting to remove the > 'ret' variable? > no, #ifdef SYS_getrandom && defined(__LINUX__) int ret; ret = syscall(SYS_getrandom, key->data, 8, 0); if (ret == 8) return; #endif > Cheers, > Benjamin > >> >> re, >> wh >> >> >>> srandom ((int)getpid() ^ time((Time_t *)0)); >>> lowbits = random (); >>> highbits = random (); >>> diff --git a/configure.ac b/configure.ac >>> index 2288502..d0d4d05 100644 >>> --- a/configure.ac >>> +++ b/configure.ac >>> @@ -63,6 +63,9 @@ case $host_os in >>> ;; >>> esac >>> >>> +# Checks for syscalls >>> +AC_CHECK_DECLS([SYS_getrandom], [], [], [[#include ]]) >>> + >>> # Checks for library functions. >>> AC_CHECK_LIB([bsd], [arc4random_buf]) >>> AC_CHECK_FUNCS([srand48 lrand48 arc4random_buf]) >> ___ >> xorg-devel@lists.x.org: X.Org development >> Archives: http://lists.x.org/archives/xorg-devel >> Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 2/2] Remove default defines of some directories.
Am 24.04.2017 22:19, schrieb Eric Anholt:
> The build defines these, so having the defaults is just a way for the
> build system's configuration to get out of sync with the code.
>
> v2: Drop #ifndefs around the other two defines.
>
> Signed-off-by: Eric Anholt
> ---
> hw/xfree86/parser/Makefile.am | 1 -
> hw/xfree86/parser/scan.c | 19 ---
> xkb/ddxLoad.c | 12
> 3 files changed, 32 deletions(-)
>
> diff --git a/hw/xfree86/parser/Makefile.am b/hw/xfree86/parser/Makefile.am
> index 9aa8cfefbd2b..2e4c6afdb21e 100644
> --- a/hw/xfree86/parser/Makefile.am
> +++ b/hw/xfree86/parser/Makefile.am
> @@ -24,7 +24,6 @@ libxf86config_la_SOURCES = \
> $(INTERNAL_SOURCES)
>
> AM_CFLAGS = $(DIX_CFLAGS) $(XORG_CFLAGS) \
> - -DSYSCONFDIR=\"$(sysconfdir)\" \
> -DDATADIR=\"$(datadir)\"
>
> EXTRA_DIST = \
> diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
> index 3356224ce061..bac213a73ab9 100644
> --- a/hw/xfree86/parser/scan.c
> +++ b/hw/xfree86/parser/scan.c
> @@ -542,27 +542,8 @@ xf86pathIsSafe(const char *path)
> *%%%
> */
>
> -#ifndef XCONFIGFILE
> -#define XCONFIGFILE "xorg.conf"
> -#endif
> -#ifndef XCONFIGDIR
> -#define XCONFIGDIR "xorg.conf.d"
> -#endif
> -#ifndef XCONFIGSUFFIX
> #define XCONFIGSUFFIX".conf"
while we are here ...
this pattern seems to be used to filter inside XCONFIGDIR for *.conf.
why not set this to "" or even better drop it all together ?
re,
wh
> -#endif
> -#ifndef PROJECTROOT
> -#define PROJECTROOT "/usr/X11R6"
> -#endif
> -#ifndef SYSCONFDIR
> -#define SYSCONFDIR PROJECTROOT "/etc"
> -#endif
> -#ifndef DATADIR
> -#define DATADIR PROJECTROOT "/share"
> -#endif
> -#ifndef XCONFENV
> #define XCONFENV "XORGCONFIG"
> -#endif
>
> #define BAIL_OUT do {
> \
> free(result);
> \
> diff --git a/xkb/ddxLoad.c b/xkb/ddxLoad.c
> index f71815aa814b..a1a0fd3a28be 100644
> --- a/xkb/ddxLoad.c
> +++ b/xkb/ddxLoad.c
> @@ -45,18 +45,6 @@ THE USE OR PERFORMANCE OF THIS SOFTWARE.
> #include
> #include "xkb.h"
>
> -/*
> - * If XKM_OUTPUT_DIR specifies a path without a leading slash, it is
> - * relative to the top-level XKB configuration directory.
> - * Making the server write to a subdirectory of that directory
> - * requires some work in the general case (install procedure
> - * has to create links to /var or somesuch on many machines),
> - * so we just compile into /usr/tmp for now.
> - */
> -#ifndef XKM_OUTPUT_DIR
> -#define XKM_OUTPUT_DIR "compiled/"
> -#endif
> -
> #define PRE_ERROR_MSG "\"The XKEYBOARD keymap compiler (xkbcomp)
> reports:\""
> #define ERROR_PREFIX"\"> \""
> #define POST_ERROR_MSG1 "\"Errors from xkbcomp are not fatal to the X
> server\""
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xfree86: up the path name size to 512 in xf86MatchDriverFromFiles
if i remember correctly asprint() was added last year.
Would that be an option ?
re,
wh
Am 05.05.2017 01:06, schrieb Peter Hutterer:
> ./hw/xfree86/common/xf86pciBus.c: In function ‘xf86MatchDriverFromFiles’:
> ./hw/xfree86/common/xf86pciBus.c:1330:52: warning: ‘snprintf’ output may be
> truncated before the last format character [-Wformat-truncation=]
> snprintf(path_name, sizeof(path_name), "%s/%s", ^~~
> ./hw/xfree86/common/xf86pciBus.c:1330:13: note: ‘snprintf’ output between 2
>
> dirent->d_name is 256, so sprintf("%s/%s") into a 256 buffer gives us:
>
> and 257 bytes into a destination of size 256
>
> Signed-off-by: Peter Hutterer
> ---
> hw/xfree86/common/xf86pciBus.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/xfree86/common/xf86pciBus.c b/hw/xfree86/common/xf86pciBus.c
> index 0d0a76bc8..012ca08d2 100644
> --- a/hw/xfree86/common/xf86pciBus.c
> +++ b/hw/xfree86/common/xf86pciBus.c
> @@ -1303,7 +1303,7 @@ xf86MatchDriverFromFiles(uint16_t match_vendor,
> uint16_t match_chip,
> char *line = NULL, *tmpMatch;
> size_t len;
> ssize_t read;
> -char path_name[256], vendor_str[5], chip_str[5];
> +char path_name[512], vendor_str[5], chip_str[5];
> uint16_t vendor, chip;
> int j;
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr 2/2] xrandr: stricter --scale argument parsing
Am 17.05.2017 18:30, schrieb Giuseppe Bilotta:
> We used to accept something like --scale 2x3junk as a valid input
> (scaling x by 2 and y by 3), even though this isn't really a valid
> scaling factor.
>
> Fix by making sure there is nothing after the parsed number(s).
>
> Signed-off-by: Giuseppe Bilotta
> ---
> xrandr.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xrandr.c b/xrandr.c
> index 9f73d0f..9673b0e 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -2981,11 +2981,12 @@ main (int argc, char **argv)
> if (!strcmp ("--scale", argv[i]))
> {
> double sx, sy;
> + char junk;
> if (!config_output) argerr ("%s must be used after --output\n",
> argv[i]);
> if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2)
> + if (sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk) != 2)
> {
> - if (sscanf (argv[i], "%lf", &sx) != 1)
> + if (sscanf (argv[i], "%lf%c", &sx, &junk) != 1)
> argerr ("failed to parse '%s' as a scaling
> factor\n", argv[i]);
> sy = sx;
> }
perhaps that is more easy:
n=sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk)
if (n==1)
sy=sx;
else if (n != 2 )
argerr ("failed to parse '%s' as a scaling factor\n", argv[i]);
just my 2 cents,
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [xproto] _X_NONNULL and C++ 11
Am 27.05.2017 11:02, schrieb Matthieu Herrb: > Hi, > > Marc Espie recently found out that the X_NONNULL macro in Xfuncproto.h > is generating spurious warnings when included in C++ code build with > clang++ -std=c++11. > > Other OpenBSD developper tried to find uses of the macro in the wild > and didn't find any, even in the X.Org lib app or xserver tree. > > So, should this macro definition be removed alltogether (acking that > no-one cares to use it) or just apply the patch below ? > > From 6ae956660879d70e078025c3d8f1ac3fd438cad2 Mon Sep 17 00:00:00 2001 > From: Marc Espie > Date: Sat, 27 May 2017 10:55:04 +0200 > Subject: [PATCH] Fix compiling any C++ code including Xfuncproto.h with > clang++ -std=c++11 > > It shouldn't warn, bu it will use the "legacy" varargs macros and whine. > --- > Xfuncproto.h.in | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/Xfuncproto.h.in b/Xfuncproto.h.in > index b88493d..1be3f55 100644 > --- a/Xfuncproto.h.in > +++ b/Xfuncproto.h.in > @@ -166,7 +166,8 @@ in this Software without prior written authorization from > The Open Group. > argument macros, must be only used inside #ifdef _X_NONNULL guards, as > many legacy X clients are compiled in C89 mode still. */ > #if __has_attribute(nonnull) \ > -&& defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L) /* C99 > */ > +&& (defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L) \ > +|| (defined(__cplusplus) && (__cplusplus - 0 >= 201103L))) /* C99 C++11 > */ > #define _X_NONNULL(...) __attribute__((nonnull(__VA_ARGS__))) > #elif __has_attribute(nonnull) \ > || defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 303) > > > So far i understand this is a problem with clang++. "spurious warning" sounds for me more like a compiler bug (assuming that gcc has no problems with that). So why not make that a NOOP until fixed in clang ? re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [xproto] _X_NONNULL and C++ 11
Am 27.05.2017 14:10, schrieb Mark Kettenis: >> Date: Sat, 27 May 2017 13:33:25 +0200 >> From: walter harms >> >> Am 27.05.2017 11:02, schrieb Matthieu Herrb: >>> Hi, >>> >>> Marc Espie recently found out that the X_NONNULL macro in Xfuncproto.h >>> is generating spurious warnings when included in C++ code build with >>> clang++ -std=c++11. >>> >>> Other OpenBSD developper tried to find uses of the macro in the wild >>> and didn't find any, even in the X.Org lib app or xserver tree. >>> >>> So, should this macro definition be removed alltogether (acking that >>> no-one cares to use it) or just apply the patch below ? >>> >>> From 6ae956660879d70e078025c3d8f1ac3fd438cad2 Mon Sep 17 00:00:00 2001 >>> From: Marc Espie >>> Date: Sat, 27 May 2017 10:55:04 +0200 >>> Subject: [PATCH] Fix compiling any C++ code including Xfuncproto.h with >>> clang++ -std=c++11 >>> >>> It shouldn't warn, bu it will use the "legacy" varargs macros and whine. >>> --- >>> Xfuncproto.h.in | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/Xfuncproto.h.in b/Xfuncproto.h.in >>> index b88493d..1be3f55 100644 >>> --- a/Xfuncproto.h.in >>> +++ b/Xfuncproto.h.in >>> @@ -166,7 +166,8 @@ in this Software without prior written authorization >>> from The Open Group. >>> argument macros, must be only used inside #ifdef _X_NONNULL guards, as >>> many legacy X clients are compiled in C89 mode still. */ >>> #if __has_attribute(nonnull) \ >>> -&& defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L) /* >>> C99 */ >>> +&& (defined(__STDC_VERSION__) && (__STDC_VERSION__ - 0 >= 199901L) \ >>> +|| (defined(__cplusplus) && (__cplusplus - 0 >= 201103L))) /* C99 >>> C++11 */ >>> #define _X_NONNULL(...) __attribute__((nonnull(__VA_ARGS__))) >>> #elif __has_attribute(nonnull) \ >>> || defined(__GNUC__) && ((__GNUC__ * 100 + __GNUC_MINOR__) >= 303) >>> >>> >>> >> >> So far i understand this is a problem with clang++. "spurious >> warning" sounds for me more like a compiler bug (assuming that gcc >> has no problems with that). So why not make that a NOOP until fixed >> in clang ? > > GCC 4.9.4 warns as well: > > $ g++ -pedantic -std=c++11 -I/usr/X11R6/include xfunc.cc > In file included from xfunc.cc:1:0: > /usr/X11R6/include/X11/Xfuncproto.h:173:24: warning: ISO C does not permit > named variadic macros [-Wvariadic-macros] > #define _X_NONNULL(args...) __attribute__((nonnull(args))) > i have checked my version and can confirm the problem also with gcc (and -pedantic). i guess it is more simple the remove the macro completely. It is only for optimization, it is never used, ... lets remove it. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Stabilizing Xdmx -- advice
Am 23.06.2017 19:50, schrieb Joshua Marshall: > Hello all, > > Xdmx is giving me a fair bit of trouble. In particular regard to > segfaulting. Now, I'm assuming all the easy to catch stuff is already > done, so where in general should I give attention to improving the state of > Xdmx? > > I would say 'scratch where it itches'', you will never know how many people will cherish it or even when. when you have patches simply read:https://www.x.org/wiki/Development/Documentation/SubmittingPatches/ and hope for the best. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen
documentation is a painful job, thx for doing this. re, wh Am 28.05.2017 23:33, schrieb Pali Rohár: > Explicitly document and make it clear that those options does not change > DPI of some monitor output. Also state that these options have no useful > meaning for multi-monitor configuration. > > Signed-off-by: Pali Rohár > --- > man/xrandr.man | 14 ++ > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/man/xrandr.man b/man/xrandr.man > index 55ea5dd..fec4a1a 100644 > --- a/man/xrandr.man > +++ b/man/xrandr.man > @@ -226,14 +226,20 @@ fit within this size. When this option is not provided, > xrandr computes the > smallest screen size that will hold the set of configured outputs; this > option provides a way to override that behaviour. > .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP" > -Sets the reported values for the physical size of the screen. Normally, > +Sets the reported values for the physical size of the whole X screen (not > particular > +output!) which consists of all configured monitors. Physical size of the X > screen does > +not have any meaning when two monitors with different DPI are configured. > Normally, > xrandr resets the reported physical size values to keep the DPI constant. > -This overrides that computation. > +This overrides that computation. This option should be calculated from DPI > +value 96 as it not have any useful meaning for multi-monitor configuration. > .IP "\-\-dpi \fIdpi\fP" > .IP "\-\-dpi \fIfrom-output\fP" > -This also sets the reported physical size values of the screen, it uses > either > +This also sets the reported physical size values of the whole X screen (not > particular > +output!) which consists of all configured monitors. Physical size of the X > screen does not > +have any meaning when two monitors with different DPI are configured. This > option uses either No number needed: "have any meaning when monitors with different DPI are configured." > the specified DPI value, or the DPI of the given output, to compute an > appropriate > -physical size using whatever pixel size will be set. > +physical size using whatever pixel size will be set. This option should be > set to > +default value 96 as it not have any useful meaning for multi-monitor > configuration. The "default value 96" is confusing me, perhaps a simple reordering may help: physical size using whatever pixel size will be set (default 96 DPI). It does not have any useful meaning for multi-monitor configuration. > .IP "\-\-newmode \fIname\fP \fImode\fP" > New modelines can be added to the server and then associated with outputs. > This option does the former. The \fImode\fP is specified using the ModeLine ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] composite: Make compIsAlternateVisual safe even if Composite is off
Am 27.07.2017 22:02, schrieb Adam Jackson: > As of ea483af9 we're calling this unconditionally from the GLX code so > the synthetic visual is in a lower select group. If Composite has been > disabled then GetCompScreen() will return NULL, and this would crash. > > Rather than force the caller to check first, just always return FALSE if > Composite is disabled (which is correct, since none of the visuals will > be synthetic in that case). > > Signed-off-by: Adam Jackson > --- > composite/compwindow.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/composite/compwindow.c b/composite/compwindow.c > index 367f23eb7..f88238146 100644 > --- a/composite/compwindow.c > +++ b/composite/compwindow.c > @@ -326,7 +326,7 @@ compIsAlternateVisual(ScreenPtr pScreen, XID visual) > CompScreenPtr cs = GetCompScreen(pScreen); > int i; > > -for (i = 0; i < cs->numAlternateVisuals; i++) > +for (i = 0; cs && i < cs->numAlternateVisuals; i++) > if (cs->alternateVisuals[i] == visual) > return TRUE; > return FALSE; IMHO this would be more readable: cs = GetCompScreen(pScreen); if (!cs) return FALSE; for()... just my 2 cents ... re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen
Sorry, i had the idea you already got my ack. Reviewed-by: Walter Harms wha...@bfs.de Am 07.08.2017 12:56, schrieb Pali Rohár: > walter, any other objections? I fixed problematic parts which you pointed. > > On Monday 24 July 2017 20:34:39 Pali Rohár wrote: >> Explicitly document and make it clear that those options do not change >> DPI of some monitor output. Also state that these options have no useful >> meaning for multi-monitor configuration. >> >> Signed-off-by: Pali Rohár >> --- >> man/xrandr.man | 14 ++ >> 1 file changed, 10 insertions(+), 4 deletions(-) >> >> diff --git a/man/xrandr.man b/man/xrandr.man >> index 65ccc2a..a5476d4 100644 >> --- a/man/xrandr.man >> +++ b/man/xrandr.man >> @@ -232,14 +232,20 @@ fit within this size. When this option is not >> provided, xrandr computes the >> smallest screen size that will hold the set of configured outputs; this >> option provides a way to override that behaviour. >> .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP" >> -Sets the reported values for the physical size of the screen. Normally, >> +Sets the reported values for the physical size of the whole X screen (not >> particular >> +output!) which consists of all configured monitors. Physical size of the X >> screen does >> +not have any meaning when monitors with different DPI are configured. >> Normally, >> xrandr resets the reported physical size values to keep the DPI constant. >> -This overrides that computation. >> +This overrides that computation. This option should be calculated from DPI >> +value 96 as it not have any useful meaning for multi-monitor configuration. >> .IP "\-\-dpi \fIdpi\fP" >> .IP "\-\-dpi \fIfrom-output\fP" >> -This also sets the reported physical size values of the screen, it uses >> either >> +This also sets the reported physical size values of the whole X screen (not >> particular >> +output!) which consists of all configured monitors. Physical size of the X >> screen does not >> +have any meaning when monitors with different DPI are configured. This >> option uses either >> the specified DPI value, or the DPI of the given output, to compute an >> appropriate >> -physical size using whatever pixel size will be set. >> +physical size using whatever pixel size will be set (default 96 DPI). It >> does not >> +have any useful meaning for multi-monitor configuration. >> .IP "\-\-newmode \fIname\fP \fImode\fP" >> New modelines can be added to the server and then associated with outputs. >> This option does the former. The \fImode\fP is specified using the ModeLine > ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/3] Make _XCloseLC thread safe.
hi,
can you provide a simple program does needs the patch to work ?
re,
wh
Am 10.08.2017 23:04, schrieb Jacek Caban:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
> Signed-off-by: Jacek Caban
> ---
> src/xlibi18n/lcWrap.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
> index 38242608..43b4d622 100644
> --- a/src/xlibi18n/lcWrap.c
> +++ b/src/xlibi18n/lcWrap.c
> @@ -324,6 +324,8 @@ _XCloseLC(
> {
> XLCdList cur, *prev;
> +_XLockMutex(_Xi18n_lock);
> +
> for (prev = &lcd_list; (cur = *prev); prev = &cur->next) {
> if (cur->lcd == lcd) {
> if (--cur->ref_count < 1) {
> @@ -339,6 +341,8 @@ _XCloseLC(
> _XlcDeInitLoader();
> loader_list = NULL;
> }
> +
> +_XUnlockMutex(_Xi18n_lock);
> }
> /*
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libICE 3/3] Make sure string is never NULL
Am 04.09.2017 13:00, schrieb Emil Velikov:
> On 7 July 2017 at 11:23, Eric Engestrom wrote:
>> `error_message` is passed in to strncpy() without any check, which
>> doesn't handle NULL itself, so let's make it a valid empty string in
>> cases where it was NULL.
>>
> Strictly speaking strdup() can fail, thus we could still end with a NULL.
> In all fairness I'm not sure how much one should bother though.
>
>
I do not think this is a problem, what the patch catches is the case
string=ProcessError();
free(string);
fatal is when the function returns a static string as seen in [PATCH libICE 2/3]
then you are lost. Perhaps the whole think can be converted to a preallocated
buffer, but that would be quit invasive and out of proportion for the problem.
Acked-by: wha...@bfs.de
re,
wh
>> Signed-off-by: Eric Engestrom
>> ---
>> src/process.c | 14 --
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/process.c b/src/process.c
>> index 1ee1ceb..30d073f 100644
>> --- a/src/process.c
>> +++ b/src/process.c
>> @@ -704,6 +704,11 @@ ProcessError (
>> invokeHandler = 1;
>> }
>>
>> + if (!errorStr)
>> + {
>> + errorStr = strdup("");
>> + }
>> +
>> errorReply->type = ICE_CONNECTION_ERROR;
>> errorReply->error_message = errorStr;
>> }
>> @@ -794,6 +799,11 @@ ProcessError (
>> invokeHandler = 1;
>> }
>>
>> + if (!errorStr)
>> + {
>> + errorStr = strdup("");
>> + }
>> +
> Skimming through the file, one could drop the curly brackets.
> Indentation also seems off, although it could be my MUA.
>
> With the above nitpicks, the series is
> Reviewed-by: Emil Velikov
>
> -Emil
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 1/2] connect.c: FIX 'iceConn' shadows a previous local, [-Wshadow]
In function 'IceOpenConnection': gcc give the following warning:
connect.c:106:11: warning: declaration of 'iceConn' shadows a previous local
[-Wshadow]
fixed by renaming 2. iceConn to iConn (and all its uses)
Signed-off-by: Walter Harms
---
src/connect.c | 34 +-
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/src/connect.c b/src/connect.c
index 276a356..5ab5498 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -103,11 +103,11 @@ IceOpenConnection (
* OK, we found a connection. Make sure we can reuse it.
*/
- IceConn iceConn = _IceConnectionObjs[i];
+ IceConn iConn = _IceConnectionObjs[i];
- if (iceConn->want_to_close || iceConn->free_asap ||
- (context && iceConn->context &&
-iceConn->context != context))
+ if (iConn->want_to_close || iConn->free_asap ||
+ (context && iConn->context &&
+iConn->context != context))
{
/* force a new connection to be created */
break;
@@ -115,20 +115,20 @@ IceOpenConnection (
if (majorOpcodeCheck)
{
- for (j = iceConn->his_min_opcode;
- j <= iceConn->his_max_opcode; j++)
+ for (j = iConn->his_min_opcode;
+ j <= iConn->his_max_opcode; j++)
{
- if (iceConn->process_msg_info[
- j - iceConn->his_min_opcode].in_use &&
- iceConn->process_msg_info[
- j - iceConn->his_min_opcode].my_opcode ==
+ if (iConn->process_msg_info[
+ j - iConn->his_min_opcode].in_use &&
+ iConn->process_msg_info[
+ j - iConn->his_min_opcode].my_opcode ==
majorOpcodeCheck)
break;
}
- if (j <= iceConn->his_max_opcode ||
- (iceConn->protosetup_to_you &&
- iceConn->protosetup_to_you->my_opcode ==
+ if (j <= iConn->his_max_opcode ||
+ (iConn->protosetup_to_you &&
+ iConn->protosetup_to_you->my_opcode ==
majorOpcodeCheck))
{
/* force a new connection to be created */
@@ -136,10 +136,10 @@ IceOpenConnection (
}
}
- iceConn->open_ref_count++;
- if (context && !iceConn->context)
- iceConn->context = context;
- return (iceConn);
+ iConn->open_ref_count++;
+ if (context && !iConn->context)
+ iConn->context = context;
+ return (iConn);
}
}
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 2/2] iceauth.c: FIX warning: unused variable 'ret' in 'arc4random_buf'
commit ff5e59f32255913bb1cdf51441b98c9107ae165b left ret outside the #if
causing a gcc warnung:
In function 'arc4random_buf':
iceauth.c:89:13: warning: unused variable 'ret' [-Wunused-variable]
fixed by moving #if 1 up
Signed-off-by: Walter Harms
---
src/iceauth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/iceauth.c b/src/iceauth.c
index de4785b..7770b7f 100644
--- a/src/iceauth.c
+++ b/src/iceauth.c
@@ -86,9 +86,9 @@ arc4random_buf (
int len
)
{
+#if HAVE_GETENTROPY
intret;
-#if HAVE_GETENTROPY
/* weak emulation of arc4random through the entropy libc */
ret = getentropy (auth, len);
if (ret == 0)
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 7/8] edid-decode: add HDR Dynamic Metadata Data Block
Am 07.09.2017 20:03, schrieb Hans Verkuil:
> From: Hans Verkuil
>
> Support this CTA-861-G data block.
>
> Signed-off-by: Hans Verkuil
> ---
> edid-decode.c | 33 +
> 1 file changed, 33 insertions(+)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index effcf777..cdba099d 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -1865,6 +1865,35 @@ cea_hdr_static_metadata_block(unsigned char *x)
> x[6], (50.0 * pow(2, x[4] / 32.0)) * pow(x[6] / 255.0, 2) /
> 100.0);
> }
>
> +static void
> +cea_hdr_dyn_metadata_block(unsigned char *x)
> +{
> +int length = x[0] & 0x1f;
> +
> +if (length < 1)
> + return;
complicated way to say if (length == 0)
> +length--;
> +x--;
> +while (length >= 3) {
> + int type_len = x[0];
> + int type = x[1] | (x[2] << 8);
> +
Do you expect that x[?] will change ? otherwise this is const
and the loop print the same data over and over again, or did i miss something ?
hope that helps,
re,
wh
> + if (length < type_len)
> + return;
> + printf("HDR Dynamic Metadata Type %d\n", type);
> + switch (type) {
> + case 1:
> + case 2:
> + case 4:
> + printf(" Version: %d\n", x[3] & 0xf);
> + break;
> + default:
> + break;
> + }
> + length -= type_len;
> +}
> +}
> +
> static void
> cea_block(unsigned char *x)
> {
> @@ -1931,6 +1960,10 @@ cea_block(unsigned char *x)
> printf("HDR static metadata data block\n");
> cea_hdr_static_metadata_block(x);
> break;
> + case 0x07:
> + printf("HDR dynamic metadata data block\n");
> + cea_hdr_dyn_metadata_block(x);
> + break;
> case 0x0d:
> printf("Video format preference data block\n");
> cea_vfpdb(x);
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 6/8] edid-decode: support HLG, decode luminance values
Am 07.09.2017 20:03, schrieb Hans Verkuil:
> From: Hans Verkuil
>
> Add support for the new CTA-861-G Hybrid Log-Gamma transfer
> function.
>
> Also decode the luminance values in the static metadata block to
> cd/m^2 values.
>
> Signed-off-by: Hans Verkuil
> ---
> Makefile | 2 +-
> edid-decode.c | 15 ++-
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 21b811ed..b698c579 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2,7 +2,7 @@ bindir ?= /usr/bin
> mandir ?= /usr/share/man
>
> edid-decode: edid-decode.c
> - $(CC) $(CFLAGS) -g -Wall -o $@ $<
> + $(CC) $(CFLAGS) -g -Wall -lm -o $@ $<
>
> clean:
> rm -f edid-decode
> diff --git a/edid-decode.c b/edid-decode.c
> index d0da48e2..effcf777 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
>
> #define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
> #define min(a, b) ((a) < (b) ? (a) : (b))
> @@ -1827,10 +1828,11 @@ static const char *eotf_map[] = {
> "Traditional gamma - SDR luminance range",
> "Traditional gamma - HDR luminance range",
> "SMPTE ST2084",
> +"Hybrid Log-Gamma",
> };
>
> static void
> -cea_hdr_metadata_block(unsigned char *x)
> +cea_hdr_static_metadata_block(unsigned char *x)
> {
> int length = x[0] & 0x1f;
> int i;
> @@ -1851,13 +1853,16 @@ cea_hdr_metadata_block(unsigned char *x)
> }
>
> if (length >= 4)
> - printf("Desired content max luminance: %d\n", x[4]);
> + printf("Desired content max luminance: %d (%.3f cd/m^2)\n",
> +x[4], 50.0 * pow(2, x[4] / 32.0));
>
> if (length >= 5)
> - printf("Desired content max frame-average luminance: %d\n", x[5]);
> + printf("Desired content max frame-average luminance: %d (%.3f
> cd/m^2)\n",
> +x[5], 50.0 * pow(2, x[5] / 32.0));
>
> if (length >= 6)
> - printf("Desired content min luminance: %d\n", x[6]);
> + printf("Desired content min luminance: %d (%.3f cd/m^2)\n",
> +x[6], (50.0 * pow(2, x[4] / 32.0)) * pow(x[6] / 255.0, 2) /
> 100.0);
are you sure that pow(x[6] / 255.0, 2) is correct ? other terms are pow( 2,
...).
re,
wh
> }
>
> static void
> @@ -1924,7 +1929,7 @@ cea_block(unsigned char *x)
> break;
> case 0x06:
> printf("HDR static metadata data block\n");
> - cea_hdr_metadata_block(x);
> + cea_hdr_static_metadata_block(x);
> break;
> case 0x0d:
> printf("Video format preference data block\n");
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 3/8] edid-decode: add support for Room/Speaker data blocks
Am 07.09.2017 20:03, schrieb Hans Verkuil:
> From: Hans Verkuil
>
> Support the Room Configuration Data Block and the Speaker Location
> Data Block.
>
> Signed-off-by: Hans Verkuil
> ---
> edid-decode.c | 79
> ---
> 1 file changed, 76 insertions(+), 3 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index 9b6c297e..bdeb0c49 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -1614,7 +1614,7 @@ static struct field *vcdb_fields[] = {
> &CE_scan,
> };
>
> -static const char *sadb_map[] = {
> +static const char *speaker_map[] = {
> "FL/FR - Front Left/Right",
> "LFE - Low Frequency Effects",
> "FC - Front Center",
> @@ -1647,13 +1647,78 @@ cea_sadb(unsigned char *x)
>
> printf("Speaker map:\n");
>
> - for (i = 0; i < ARRAY_SIZE(sadb_map); i++) {
> + for (i = 0; i < ARRAY_SIZE(speaker_map); i++) {
> if ((sad >> i) & 1)
> - printf(" %s\n", sadb_map[i]);
> + printf(" %s\n", speaker_map[i]);
> }
> }
> }
>
> +static float
> +uchar_to_float(unsigned char x)
> +{
> +signed char s = (signed char)x;
> +
> +return s / 64.0;
> +}
The Name is a bit unfortunate as there is more than a type change
to be fair i have no better name, to_64flt() ?
re,
wh
> +
> +static void
> +cea_rcdb(unsigned char *x)
> +{
> +int length = x[0] & 0x1f;
> +uint32_t spm = ((x[5] << 16) | (x[4] << 8) | x[3]);
> +int i;
> +
> +if (length < 12)
> + return;
> +
> +if (x[2] & 0x40)
> + printf("Speaker count: %d\n", x[2] & 0x1f);
> +
> +printf("Speaker Presence Mask:\n");
> +for (i = 0; i < ARRAY_SIZE(speaker_map); i++) {
> + if ((spm >> i) & 1)
> + printf(" %s\n", speaker_map[i]);
> +}
> +if (x[2] & 0x20) {
> + printf("Xmax: %d dm\n", x[6]);
> + printf("Ymax: %d dm\n", x[7]);
> + printf("Zmax: %d dm\n", x[8]);
> +}
> +if (x[2] & 0x80) {
> + printf("DisplayX: %.3f * Xmax\n", uchar_to_float(x[9]));
> + printf("DisplayY: %.3f * Ymax\n", uchar_to_float(x[10]));
> + printf("DisplayZ: %.3f * Zmax\n", uchar_to_float(x[11]));
> +}
> +}
> +
> +static void
> +cea_sldb(unsigned char *x)
> +{
> +int length = x[0] & 0x1f;
> +
> +if (length < 2)
> + return;
> +
> +x += 2;
> +length -= 2;
> +
> +while (length >= 2) {
> + printf("Channel: %d (%sactive)\n", x[0] & 0x1f,
> +(x[0] & 0x20) ? "" : "not ");
> + if ((x[1] & 0x1f) < ARRAY_SIZE(speaker_map))
> + printf("Speaker: %s\n", speaker_map[x[1] & 0x1f]);
> + if (length >= 5 && (x[0] & 0x40)) {
> + printf("X: %.3f * Xmax\n", uchar_to_float(x[2]));
> + printf("Y: %.3f * Ymax\n", uchar_to_float(x[3]));
> + printf("Z: %.3f * Zmax\n", uchar_to_float(x[4]));
> + length -= 3;
> + }
> +
> + length -= 2;
> +}
> +}
> +
> static void
> cea_vcdb(unsigned char *x)
> {
> @@ -1811,6 +1876,14 @@ cea_block(unsigned char *x)
> case 0x12:
> printf("HDMI audio data block\n");
> break;
> + case 0x13:
> + printf("Room configuration data block\n");
> + cea_rcdb(x);
> + break;
> + case 0x14:
> + printf("Speaker location data block\n");
> + cea_sldb(x);
> + break;
> case 0x20:
> printf("InfoFrame data block\n");
> break;
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH 2/8] edid-decode: update Speaker Allocation data block
Am 07.09.2017 20:03, schrieb Hans Verkuil:
> From: Hans Verkuil
>
> More bits are now in use, implement support for those.
>
> Signed-off-by: Hans Verkuil
> ---
> edid-decode.c | 38 ++
> 1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index 26550423..9b6c297e 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -1615,17 +1615,25 @@ static struct field *vcdb_fields[] = {
> };
>
> static const char *sadb_map[] = {
> -"FL/FR",
> -"LFE",
> -"FC",
> -"RL/RR",
> -"RC",
> -"FLC/FRC",
> -"RLC/RRC",
> -"FLW/FRW",
> -"FLH/FRH",
> -"TC",
> -"FCH",
> +"FL/FR - Front Left/Right",
> +"LFE - Low Frequency Effects",
> +"FC - Front Center",
> +"BL/BR - Back Left/Right",
> +"BC - Back Center",
> +"FLC/FRC - Front Left/Right of Center",
> +"RLC/RRC - Rear Left/Right of Center",
> +"FLW/FRW - Front Left/Right Wide",
> +"TpFL/TpFH - Top Front Left/Right",
> +"TpC - Top Center",
> +"LS/RS - Left/Right Surround",
> +"LFE2 - Low Frequency Effects 2",
> +"TpBC - Top Back Center",
> +"SiL/SiR - Side Left/Right",
> +"TpSiL/TpSiR - Top Side Left/Right",
> +"TpBL/TpBR - Top Back Left/Right",
> +"BtFC - Bottom Front Center",
> +"BtFL/BtBR - Bottom Front Left/Right",
> +"TpLS/TpRS - Top Left/Right Surround",
> };
>
> static void
> @@ -1635,16 +1643,14 @@ cea_sadb(unsigned char *x)
> int i;
>
> if (length >= 3) {
> - uint16_t sad = ((x[2] << 8) | x[1]);
> + uint32_t sad = ((x[3] << 16) | (x[2] << 8) | x[1]);
>
> - printf("Speaker map:");
> + printf("Speaker map:\n");
>
> for (i = 0; i < ARRAY_SIZE(sadb_map); i++) {
> if ((sad >> i) & 1)
> - printf(" %s", sadb_map[i]);
> + printf(" %s\n", sadb_map[i]);
> }
just a remark:
if (sad & 1)
sad >>=1 ;
if (sad == 0) break;
to avoid testing empty bits ..
re,
wh
> -
> - printf("\n");
> }
> }
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCHv2 4/8] edid-decode: add support for Room/Speaker data blocks
Am 08.09.2017 12:32, schrieb Hans Verkuil:
> From: Hans Verkuil
>
> Support the Room Configuration Data Block and the Speaker Location
> Data Block.
>
> Signed-off-by: Hans Verkuil
> ---
> edid-decode.c | 112
> --
> 1 file changed, 109 insertions(+), 3 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index 8d47a5fc..4534d58f 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -1614,7 +1614,7 @@ static struct field *vcdb_fields[] = {
> &CE_scan,
> };
>
> -static const char *sadb_map[] = {
> +static const char *speaker_map[] = {
> "FL/FR - Front Left/Right",
> "LFE - Low Frequency Effects",
> "FC - Front Center",
> @@ -1648,13 +1648,111 @@ cea_sadb(unsigned char *x)
>
> printf("Speaker map:\n");
>
> - for (i = 0; i < ARRAY_SIZE(sadb_map); i++) {
> + for (i = 0; i < ARRAY_SIZE(speaker_map); i++) {
> if ((sad >> i) & 1)
> - printf(" %s\n", sadb_map[i]);
> + printf(" %s\n", speaker_map[i]);
> }
> }
> }
>
> +static float
> +decode_uchar_as_float(unsigned char x)
> +{
> +signed char s = (signed char)x;
> +
> +return s / 64.0;
> +}
> +
> +static void
> +cea_rcdb(unsigned char *x)
> +{
> +int length = x[0] & 0x1f;
> +uint32_t spm = ((x[5] << 16) | (x[4] << 8) | x[3]);
> +int i;
> +
> +if (length < 4)
> + return;
> +
> +if (x[2] & 0x40)
> + printf("Speaker count: %d\n", (x[2] & 0x1f) + 1);
> +
> +printf("Speaker Presence Mask:\n");
> +for (i = 0; i < ARRAY_SIZE(speaker_map); i++) {
> + if ((spm >> i) & 1)
> + printf(" %s\n", speaker_map[i]);
> +}
> +if ((x[2] & 0x20) && length >= 7) {
> + printf("Xmax: %d dm\n", x[6]);
> + printf("Ymax: %d dm\n", x[7]);
> + printf("Zmax: %d dm\n", x[8]);
length >= 7) ... x[8]);
off by one ?
> +}
> +if ((x[2] & 0x80) && length >= 10) {
> + printf("DisplayX: %.3f * Xmax\n", decode_uchar_as_float(x[9]));
> + printf("DisplayY: %.3f * Ymax\n", decode_uchar_as_float(x[10]));
> + printf("DisplayZ: %.3f * Zmax\n", decode_uchar_as_float(x[11]));
off by one ?
> +}
> +}
> +
> +static const char *speaker_location[] = {
> +"FL - Front Left",
> +"FR - Front Right",
> +"FC - Front Center",
> +"LFE1 - Low Frequency Effects 1",
> +"BL - Back Left",
> +"BR - Back Right",
> +"FLC - Front Left of Center",
> +"FRC - Front Right of Center",
> +"BC - Back Center",
> +"LFE2 - Low Frequency Effects 2",
> +"SiL - Side Left",
> +"SiR - Side Right",
> +"TpFL - Top Front Left",
> +"TpFH - Top Front Right",
> +"TpFC - Top Front Center",
> +"TpC - Top Center",
> +"TpBL - Top Back Left",
> +"TpBR - Top Back Right",
> +"TpSiL - Top Side Left",
> +"TpSiR - Top Side Right",
> +"TpBC - Top Back Center",
> +"BtFC - Bottom Front Center",
> +"BtFL - Bottom Front Left",
> +"BtBR - Bottom Front Right",
> +"FLW - Front Left Wide",
> +"FRW - Front Right Wide",
> +"LS - Left Surround",
> +"RS - Right Surround",
> +};
> +
> +static void
> +cea_sldb(unsigned char *x)
> +{
> +int length = x[0] & 0x1f;
> +
> +if (!length)
> + return;
> +
> +x += 2;
> +length--;
> +
> +while (length >= 2) {
> + printf("Channel: %d (%sactive)\n", x[0] & 0x1f,
> +(x[0] & 0x20) ? "" : "not ");
> + if ((x[1] & 0x1f) < ARRAY_SIZE(speaker_location))
> + printf(" Speaker: %s\n", speaker_location[x[1] & 0x1f]);
> + if (length >= 5 && (x[0] & 0x40)) {
> + printf(" X: %.3f * Xmax\n", decode_uchar_as_float(x[2]));
> + printf(" Y: %.3f * Ymax\n", decode_uchar_as_float(x[3]));
> + printf(" Z: %.3f * Zmax\n", decode_uchar_as_float(x[4]));
> + length -= 3;
> + x += 3;
> + }
> +
> + length -= 2;
> + x += 2;
> +}
> +}
> +
> static void
> cea_vcdb(unsigned char *x)
> {
> @@ -1812,6 +1910,14 @@ cea_block(unsigned char *x)
> case 0x12:
> printf("HDMI audio data block\n");
> break;
> + case 0x13:
> + printf("Room configuration data block\n");
> + cea_rcdb(x);
> + break;
> + case 0x14:
> + printf("Speaker location data block\n");
> + cea_sldb(x);
> + break;
> case 0x20:
> printf("InfoFrame data block\n");
> break;
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCHv2 5/8] edid-decode: add new CTA-861-G VIC codes
Am 08.09.2017 12:32, schrieb Hans Verkuil:
> From: Hans Verkuil
>
> The CTA-861-G standard (successor to CEA-861-F) adds new VIC codes.
>
> Signed-off-by: Hans Verkuil
> ---
> edid-decode.c | 93
> +++
> 1 file changed, 81 insertions(+), 12 deletions(-)
>
> diff --git a/edid-decode.c b/edid-decode.c
> index 4534d58f..074cb821 100644
> --- a/edid-decode.c
> +++ b/edid-decode.c
> @@ -1113,10 +1113,12 @@ cea_audio_block(unsigned char *x)
> }
> }
>
> -static struct {
> +struct edid_cea_mode {
> const char *name;
> int refresh, hor_freq_hz, pixclk_khz;
> -} edid_cea_modes[] = {
> +};
> +
> +static struct edid_cea_mode edid_cea_modes1[] = {
> /* VIC 1 */
> {"640x480@60Hz 4:3", 60, 31469, 25175},
> {"720x480@60Hz 4:3", 60, 31469, 27000},
> @@ -1235,14 +1237,80 @@ static struct {
> {"3840x2160@30Hz 64:27", 30, 67500, 297000},
> {"3840x2160@50Hz 64:27", 50, 112500, 594000},
> {"3840x2160@60Hz 64:27", 60, 135000, 594000},
> +{"1280x720@48Hz 16:9", 48, 36000, 9},
> +{"1280x720@48Hz 64:27", 48, 36000, 9},
> +{"1680x720@48Hz 64:27", 48, 36000, 99000},
> +/* VIC 111 */
> +{"1920x1080@48Hz 16:9", 48, 54000, 148500},
> +{"1920x1080@48Hz 64:27", 48, 54000, 148500},
> +{"2560x1080@48Hz 64:27", 48, 52800, 198000},
> +{"3840x2160@48Hz 16:9", 48, 108000, 594000},
> +{"4096x2160@48Hz 256:135", 48, 108000, 594000},
> +{"3840x2160@48Hz 64:27", 48, 108000, 594000},
> +{"3840x2160@100Hz 16:9", 100, 225000, 1188000},
> +{"3840x2160@120Hz 16:9", 120, 27, 1188000},
> +{"3840x2160@100Hz 64:27", 100, 225000, 1188000},
> +{"3840x2160@120Hz 64:27", 120, 27, 1188000},
> +/* VIC 121 */
> +{"5120x2160@24Hz 64:27", 24, 52800, 396000},
> +{"5120x2160@25Hz 64:27", 25, 55000, 396000},
> +{"5120x2160@30Hz 64:27", 30, 66000, 396000},
> +{"5120x2160@48Hz 64:27", 48, 118800, 742500},
> +{"5120x2160@50Hz 64:27", 50, 112500, 742500},
> +{"5120x2160@60Hz 64:27", 60, 135000, 742500},
> +{"5120x2160@100Hz 64:27", 100, 225000, 1485000},
> +};
> +
> +static struct edid_cea_mode edid_cea_modes2[] = {
> +/* VIC 193 */
> +{"5120x2160@120Hz 64:27", 120, 27, 1485000},
> +{"7680x4320@24Hz 16:9", 24, 108000, 1188000},
> +{"7680x4320@25Hz 16:9", 25, 11, 1188000},
> +{"7680x4320@30Hz 16:9", 30, 132000, 1188000},
> +{"7680x4320@48Hz 16:9", 48, 216000, 2376000},
> +{"7680x4320@50Hz 16:9", 50, 22, 2376000},
> +{"7680x4320@60Hz 16:9", 60, 264000, 2376000},
> +{"7680x4320@100Hz 16:9", 100, 45, 4752000},
> +/* VIC 201 */
> +{"7680x4320@120Hz 16:9", 120, 54, 4752000},
> +{"7680x4320@24Hz 64:27", 24, 108000, 1188000},
> +{"7680x4320@25Hz 64:27", 25, 11, 1188000},
> +{"7680x4320@30Hz 64:27", 30, 132000, 1188000},
> +{"7680x4320@48Hz 64:27", 48, 216000, 2376000},
> +{"7680x4320@50Hz 64:27", 50, 22, 2376000},
> +{"7680x4320@60Hz 64:27", 60, 264000, 2376000},
> +{"7680x4320@100Hz 64:27", 100, 45, 4752000},
> +{"7680x4320@120Hz 64:27", 120, 54, 4752000},
> +{"10240x4320@24Hz 64:27", 24, 118800, 1485000},
> +/* VIC 211 */
> +{"10240x4320@25Hz 64:27", 25, 11, 1485000},
> +{"10240x4320@30Hz 64:27", 30, 135000, 1485000},
> +{"10240x4320@48Hz 64:27", 48, 237600, 297},
> +{"10240x4320@50Hz 64:27", 50, 22, 297},
> +{"10240x4320@60Hz 64:27", 60, 27, 297},
> +{"10240x4320@100Hz 64:27", 100, 45, 594},
> +{"10240x4320@120Hz 64:27", 120, 54, 594},
> +{"4096x2160@100Hz 256:135", 100, 225000, 1188000},
> +{"4096x2160@120Hz 256:135", 120, 27, 1188000},
> };
>
> +static const struct edid_cea_mode *
> +vic_to_mode(unsigned char vic)
> +{
> +if (vic > 0 && vic <= ARRAY_SIZE(edid_cea_modes1))
> + return edid_cea_modes1 + vic - 1;
> +if (vic >= 193 && vic <= ARRAY_SIZE(edid_cea_modes2) + 193)
> + return edid_cea_modes2 + vic - 193;
> +return NULL;
> +}
> +
I do not see details yet, but maybe you can simply the code by
returning a dummy struct edid_cea_mode [0] =
{"Unknown mode 0:0", 0, 0, 0}
> static void
> cea_svd(unsigned char *x, int n, int for_ycbcr420)
> {
> int i;
>
> for (i = 0; i < n; i++) {
> + const struct edid_cea_mode *vicmode = NULL;
> unsigned char svd = x[i];
> unsigned char native;
> unsigned char vic;
> @@ -1261,7 +1329,8 @@ cea_svd(unsigned char *x, int n, int for_ycbcr420)
> native = svd & 0x80;
> }
>
> - if (vic > 0 && vic <= ARRAY_SIZE(edid_cea_modes)) {
> + vicmode = vic_to_mode(vic);
> + if (vicmode) {
> switch (vic) {
> case 95:
> supported_hdmi_vic_vsb_codes |= 1 << 0;
> @@ -1276,13 +1345,13 @@ cea_svd(unsigned char *x, int n, int for_ycbcr420)
> supported_hdmi_vic_vsb_codes |= 1 << 3;
> break;
>
[PATCH libICE 1/2] free() can handle NULL so remove the check
free() can handle NULL so remove the check
Signed-off-by: Walter Harms
---
src/authutil.c | 24
src/misc.c | 3 +--
src/process.c | 41 +++--
src/shutdown.c | 44 +++-
4 files changed, 39 insertions(+), 73 deletions(-)
diff --git a/src/authutil.c b/src/authutil.c
index ca0504a..d7bcef9 100644
--- a/src/authutil.c
+++ b/src/authutil.c
@@ -111,8 +111,8 @@ IceAuthFileName (void)
if (size > bsize)
{
- if (buf)
- free (buf);
+
+ free (buf);
buf = malloc (size);
if (!buf) {
bsize = 0;
@@ -266,11 +266,11 @@ IceReadAuthFileEntry (
bad:
-if (local.protocol_name) free (local.protocol_name);
-if (local.protocol_data) free (local.protocol_data);
-if (local.network_id) free (local.network_id);
-if (local.auth_name) free (local.auth_name);
-if (local.auth_data) free (local.auth_data);
+free (local.protocol_name);
+free (local.protocol_data);
+free (local.network_id);
+free (local.auth_name);
+free (local.auth_data);
return (NULL);
}
@@ -284,11 +284,11 @@ IceFreeAuthFileEntry (
{
if (auth)
{
- if (auth->protocol_name) free (auth->protocol_name);
- if (auth->protocol_data) free (auth->protocol_data);
- if (auth->network_id) free (auth->network_id);
- if (auth->auth_name) free (auth->auth_name);
- if (auth->auth_data) free (auth->auth_data);
+ free (auth->protocol_name);
+ free (auth->protocol_data);
+ free (auth->network_id);
+ free (auth->auth_name);
+ free (auth->auth_data);
free (auth);
}
}
diff --git a/src/misc.c b/src/misc.c
index d2e9150..87d6335 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -54,8 +54,7 @@ IceAllocScratch (
{
if (!iceConn->scratch || size > iceConn->scratch_size)
{
- if (iceConn->scratch)
- free (iceConn->scratch);
+ free (iceConn->scratch);
iceConn->scratch = malloc (size);
iceConn->scratch_size = size;
diff --git a/src/process.c b/src/process.c
index 4100a83..a9a8d08 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1026,8 +1026,7 @@ ProcessConnectionSetup (
iceConn->connection_status = IceConnectRejected;
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
if (iceConn->connection_status == IceConnectRejected)
@@ -1080,8 +1079,7 @@ ProcessConnectionSetup (
if (authData && authDataLen > 0)
free (authData);
- if (errorString)
- free (errorString);
+ free (errorString);
}
if (accept_setup_now)
@@ -1369,8 +1367,7 @@ ProcessAuthReply (
status = IcePaAuthAccepted;
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
if (status != IcePaAuthAccepted)
@@ -1444,8 +1441,7 @@ ProcessAuthReply (
status = IcePaAuthAccepted;
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
if (status == IcePaAuthRejected)
@@ -1559,18 +1555,15 @@ ProcessAuthReply (
_IceErrorSetupFailed (iceConn, ICE_ProtocolSetup,
failureReason);
- if (failureReason)
- free (failureReason);
+ free (failureReason);
}
}
if (free_setup_info)
{
- if (iceConn->protosetup_to_me->his_vendor)
- free (iceConn->protosetup_to_me->his_vendor);
- if (iceConn->protosetup_to_me->his_release)
- free (iceConn->protosetup_to_me->his_release);
+ free (iceConn->protosetup_to_me->his_vendor);
+ free (iceConn->protosetup_to_me->his_release);
free (iceConn->protosetup_to_me);
iceConn->protosetup_to_me = NULL;
}
@@ -1587,8 +1580,8 @@ ProcessAuthReply (
if (authData && authDataLen > 0)
free (authData);
-if (errorString)
- free (errorString);
+
+free (errorString);
IceDisposeCompleteMessage (iceConn, replyData);
return (0);
@@ -2071,8 +2064,7 @@ ProcessProtocolSetup (
ICE_ProtocolSetup, "None of the authentication protocols
specified are supported and host-based authentication failed");
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
}
else
@@ -2118,8 +2110,8 @@ ProcessProtocolSetup (
if (authData && authDataLen > 0)
free (authData);
- if (errorString)
- free (errorString);
+
+ free (errorStri
[PATCH libICE 2/2] make IceProtocolShutdown() more readable
I found IceProtocolShutdown() hard to read only to find that was
it does it aktually very simple. So i rearranged the code to make
it more readable.
Signed-off-by: Walter Harms
---
src/shutdown.c | 47 ---
1 file changed, 20 insertions(+), 27 deletions(-)
diff --git a/src/shutdown.c b/src/shutdown.c
index 90e9ded..98376a7 100644
--- a/src/shutdown.c
+++ b/src/shutdown.c
@@ -40,45 +40,38 @@ IceProtocolShutdown (
int majorOpcode
)
{
+int i;
+
if (iceConn->proto_ref_count == 0 || iceConn->process_msg_info == NULL ||
majorOpcode < 1 || majorOpcode > _IceLastMajorOpcode)
{
return (0);
}
-else
-{
- /*
-* Make sure this majorOpcode is really being used.
-*/
-
- int i;
+
+
+/*
+ * Make sure this majorOpcode is really being used.
+ */
- for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
- {
- if (iceConn->process_msg_info[
- i - iceConn->his_min_opcode].in_use &&
-iceConn->process_msg_info[
- i - iceConn->his_min_opcode].my_opcode == majorOpcode)
- break;
- }
+for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
+ {
+ int n=i - iceConn->his_min_opcode;
+ if (iceConn->process_msg_info[n].in_use &&
+ iceConn->process_msg_info[n].my_opcode == majorOpcode)
+ {
- if (i > iceConn->his_max_opcode)
- {
- return (0);
- }
- else
- {
/*
* OK, we can shut down the protocol.
*/
- iceConn->process_msg_info[
- i - iceConn->his_min_opcode].in_use = False;
- iceConn->proto_ref_count--;
+ iceConn->process_msg_info[n].in_use = False;
+ iceConn->proto_ref_count--;
+ return (1);
+ }
+
+ }
- return (1);
- }
-}
+return (0);
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/3] Make _XCloseLC thread safe.
Am 14.08.2017 19:27, schrieb Jacek Caban:
> Hi,
>
> Sure, a simple test case is attached. It replicates what Wine does when
> initializing new threads (see [1] for Wine reports).
>
> Thanks,
> Jacek
>
> [1] https://bugs.winehq.org/show_bug.cgi?id=35041
>
i was able to get a double free bug. Is that what is expected ?
(the bugreport shows several backtraces, i want to make sure).
re,
wh
>
> On 14.08.2017 15:43, walter harms wrote:
>> hi,
>> can you provide a simple program does needs the patch to work ?
>>
>> re,
>> wh
>>
>> Am 10.08.2017 23:04, schrieb Jacek Caban:
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
>>> Signed-off-by: Jacek Caban
>>> ---
>>> src/xlibi18n/lcWrap.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
>>> index 38242608..43b4d622 100644
>>> --- a/src/xlibi18n/lcWrap.c
>>> +++ b/src/xlibi18n/lcWrap.c
>>> @@ -324,6 +324,8 @@ _XCloseLC(
>>> {
>>> XLCdList cur, *prev;
>>> +_XLockMutex(_Xi18n_lock);
>>> +
>>> for (prev = &lcd_list; (cur = *prev); prev = &cur->next) {
>>> if (cur->lcd == lcd) {
>>> if (--cur->ref_count < 1) {
>>> @@ -339,6 +341,8 @@ _XCloseLC(
>>> _XlcDeInitLoader();
>>> loader_list = NULL;
>>> }
>>> +
>>> +_XUnlockMutex(_Xi18n_lock);
>>> }
>>> /*
>
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libX11 1/3] Make _XCloseLC thread safe.
Hi Jacek,
while it worked in the latest release of libX11 i was unable to
replicate the crash with the current git version.
Can you confirm my observation ?
re,
wh
Am 14.08.2017 19:27, schrieb Jacek Caban:
> Hi,
>
> Sure, a simple test case is attached. It replicates what Wine does when
> initializing new threads (see [1] for Wine reports).
>
> Thanks,
> Jacek
>
> [1] https://bugs.winehq.org/show_bug.cgi?id=35041
>
>
> On 14.08.2017 15:43, walter harms wrote:
>> hi,
>> can you provide a simple program does needs the patch to work ?
>>
>> re,
>> wh
>>
>> Am 10.08.2017 23:04, schrieb Jacek Caban:
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=55678
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=68538
>>> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=69088
>>> Signed-off-by: Jacek Caban
>>> ---
>>> src/xlibi18n/lcWrap.c | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/src/xlibi18n/lcWrap.c b/src/xlibi18n/lcWrap.c
>>> index 38242608..43b4d622 100644
>>> --- a/src/xlibi18n/lcWrap.c
>>> +++ b/src/xlibi18n/lcWrap.c
>>> @@ -324,6 +324,8 @@ _XCloseLC(
>>> {
>>> XLCdList cur, *prev;
>>> +_XLockMutex(_Xi18n_lock);
>>> +
>>> for (prev = &lcd_list; (cur = *prev); prev = &cur->next) {
>>> if (cur->lcd == lcd) {
>>> if (--cur->ref_count < 1) {
>>> @@ -339,6 +341,8 @@ _XCloseLC(
>>> _XlcDeInitLoader();
>>> loader_list = NULL;
>>> }
>>> +
>>> +_XUnlockMutex(_Xi18n_lock);
>>> }
>>> /*
>
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libICE 1/2] free() can handle NULL so remove the check
Am 12.09.2017 10:58, schrieb Eric Engestrom:
> On Friday, 2017-09-08 19:59:17 +0200, walter harms wrote:
>> free() can handle NULL so remove the check
>
> Did you use a cocci script [1] to generate this?
> If so, can you add it to the commit message?
>
no, i used smatch, to be fair i did not mention it.
re,
wh
> Regardless, I double-checked it and it looks good to me:
> Reviewed-by: Eric Engestrom
>
> [1] perhaps something like this?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/free/ifnullfree.cocci
>
>>
>> Signed-off-by: Walter Harms
>> ---
>> src/authutil.c | 24
>> src/misc.c | 3 +--
>> src/process.c | 41 +++--
>> src/shutdown.c | 44 +++-
>> 4 files changed, 39 insertions(+), 73 deletions(-)
>>
>> diff --git a/src/authutil.c b/src/authutil.c
>> index ca0504a..d7bcef9 100644
>> --- a/src/authutil.c
>> +++ b/src/authutil.c
>> @@ -111,8 +111,8 @@ IceAuthFileName (void)
>>
>> if (size > bsize)
>> {
>> -if (buf)
>> -free (buf);
>> +
>> +free (buf);
>> buf = malloc (size);
>> if (!buf) {
>> bsize = 0;
>> @@ -266,11 +266,11 @@ IceReadAuthFileEntry (
>>
>> bad:
>>
>> -if (local.protocol_name) free (local.protocol_name);
>> -if (local.protocol_data) free (local.protocol_data);
>> -if (local.network_id) free (local.network_id);
>> -if (local.auth_name) free (local.auth_name);
>> -if (local.auth_data) free (local.auth_data);
>> +free (local.protocol_name);
>> +free (local.protocol_data);
>> +free (local.network_id);
>> +free (local.auth_name);
>> +free (local.auth_data);
>>
>> return (NULL);
>> }
>> @@ -284,11 +284,11 @@ IceFreeAuthFileEntry (
>> {
>> if (auth)
>> {
>> -if (auth->protocol_name) free (auth->protocol_name);
>> -if (auth->protocol_data) free (auth->protocol_data);
>> -if (auth->network_id) free (auth->network_id);
>> -if (auth->auth_name) free (auth->auth_name);
>> -if (auth->auth_data) free (auth->auth_data);
>> +free (auth->protocol_name);
>> +free (auth->protocol_data);
>> +free (auth->network_id);
>> +free (auth->auth_name);
>> +free (auth->auth_data);
>> free (auth);
>> }
>> }
>> diff --git a/src/misc.c b/src/misc.c
>> index d2e9150..87d6335 100644
>> --- a/src/misc.c
>> +++ b/src/misc.c
>> @@ -54,8 +54,7 @@ IceAllocScratch (
>> {
>> if (!iceConn->scratch || size > iceConn->scratch_size)
>> {
>> -if (iceConn->scratch)
>> -free (iceConn->scratch);
>> +free (iceConn->scratch);
>>
>> iceConn->scratch = malloc (size);
>> iceConn->scratch_size = size;
>> diff --git a/src/process.c b/src/process.c
>> index 4100a83..a9a8d08 100644
>> --- a/src/process.c
>> +++ b/src/process.c
>> @@ -1026,8 +1026,7 @@ ProcessConnectionSetup (
>> iceConn->connection_status = IceConnectRejected;
>> }
>>
>> -if (hostname)
>> -free (hostname);
>> +free (hostname);
>> }
>>
>> if (iceConn->connection_status == IceConnectRejected)
>> @@ -1080,8 +1079,7 @@ ProcessConnectionSetup (
>> if (authData && authDataLen > 0)
>> free (authData);
>>
>> -if (errorString)
>> -free (errorString);
>> +free (errorString);
>> }
>>
>> if (accept_setup_now)
>> @@ -1369,8 +1367,7 @@ ProcessAuthReply (
>> status = IcePaAuthAccepted;
>> }
>>
>> -if (hostname)
>> -free (hostname);
>> +free (hostname);
>> }
>>
>> if (status != IcePaAuthAccepted)
>> @@ -1444,8 +1441,7 @@ ProcessAuthReply (
>> status = IcePaAuthAccepted;
>> }
>>
>> -if (hostname)
>> -free (hostname);
>> +free (hostname);
>> }
>>
>> if (status == IcePaAuthRejected)
>> @@ -1559,18 +1555,15 @@ ProcessAuthReply (
>> _IceErrorSetupFailed (iceConn, ICE_ProtocolSetup,
>> failureReaso
Re: [PATCH libICE 2/2] make IceProtocolShutdown() more readable
Am 12.09.2017 11:27, schrieb Eric Engestrom:
> On Friday, 2017-09-08 20:03:03 +0200, walter harms wrote:
>> I found IceProtocolShutdown() hard to read only to find that was
>> it does it aktually very simple. So i rearranged the code to make
>> it more readable.
>>
>> Signed-off-by: Walter Harms
>> ---
>> src/shutdown.c | 47 ---
>> 1 file changed, 20 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/shutdown.c b/src/shutdown.c
>> index 90e9ded..98376a7 100644
>> --- a/src/shutdown.c
>> +++ b/src/shutdown.c
>> @@ -40,45 +40,38 @@ IceProtocolShutdown (
>> int majorOpcode
>> )
>> {
>> +int i;
>> +
>> if (iceConn->proto_ref_count == 0 || iceConn->process_msg_info == NULL
>> ||
>> majorOpcode < 1 || majorOpcode > _IceLastMajorOpcode)
>> {
>> return (0);
>> }
>> -else
>> -{
>> -/*
>> - * Make sure this majorOpcode is really being used.
>> - */
>> -
>> -int i;
>> +
>> +
>> +/*
>> + * Make sure this majorOpcode is really being used.
>> + */
>>
>> -for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
>> -{
>> -if (iceConn->process_msg_info[
>> -i - iceConn->his_min_opcode].in_use &&
>> -iceConn->process_msg_info[
>> -i - iceConn->his_min_opcode].my_opcode == majorOpcode)
>> -break;
>> -}
>> +for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
>> + {
>> +int n=i - iceConn->his_min_opcode;
>
> spaces around `=`, and can you be more consistent with the indentation?
> (mostly avoid mixing tabs and spaces).
i can fix that, no problem.
tx for reviewing it
re,
wh
>
> With that fixed:
> Reviewed-by: Eric Engestrom
>
>> +if (iceConn->process_msg_info[n].in_use &&
>> +iceConn->process_msg_info[n].my_opcode == majorOpcode)
>> + {
>>
>> -if (i > iceConn->his_max_opcode)
>> -{
>> -return (0);
>> -}
>> -else
>> -{
>> /*
>> * OK, we can shut down the protocol.
>> */
>>
>> -iceConn->process_msg_info[
>> -i - iceConn->his_min_opcode].in_use = False;
>> -iceConn->proto_ref_count--;
>> + iceConn->process_msg_info[n].in_use = False;
>> + iceConn->proto_ref_count--;
>> + return (1);
>> + }
>> +
>> + }
>>
>> -return (1);
>> -}
>> -}
>> +return (0);
>> }
>>
>>
>> --
>> 2.1.4
>>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 00/13] clean up patches for LibICE
Hi List, this is a bunch of chean up patches for the Inter Client Exchange library for X11. there are no functional changes just clean up like malloc checks, release of memory in error case etc. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 01/13] free() can handle NULL so remove the check
From f6d630eda04e02713da059519f56dd2dd75a703c Mon Sep 17 00:00:00 2001
free() can handle NULL so remove the check
---
src/authutil.c | 24
src/misc.c | 3 +--
src/process.c | 41 +++--
src/shutdown.c | 44 +++-
4 files changed, 39 insertions(+), 73 deletions(-)
diff --git a/src/authutil.c b/src/authutil.c
index ca0504a..d7bcef9 100644
--- a/src/authutil.c
+++ b/src/authutil.c
@@ -111,8 +111,8 @@ IceAuthFileName (void)
if (size > bsize)
{
- if (buf)
- free (buf);
+
+ free (buf);
buf = malloc (size);
if (!buf) {
bsize = 0;
@@ -266,11 +266,11 @@ IceReadAuthFileEntry (
bad:
-if (local.protocol_name) free (local.protocol_name);
-if (local.protocol_data) free (local.protocol_data);
-if (local.network_id) free (local.network_id);
-if (local.auth_name) free (local.auth_name);
-if (local.auth_data) free (local.auth_data);
+free (local.protocol_name);
+free (local.protocol_data);
+free (local.network_id);
+free (local.auth_name);
+free (local.auth_data);
return (NULL);
}
@@ -284,11 +284,11 @@ IceFreeAuthFileEntry (
{
if (auth)
{
- if (auth->protocol_name) free (auth->protocol_name);
- if (auth->protocol_data) free (auth->protocol_data);
- if (auth->network_id) free (auth->network_id);
- if (auth->auth_name) free (auth->auth_name);
- if (auth->auth_data) free (auth->auth_data);
+ free (auth->protocol_name);
+ free (auth->protocol_data);
+ free (auth->network_id);
+ free (auth->auth_name);
+ free (auth->auth_data);
free (auth);
}
}
diff --git a/src/misc.c b/src/misc.c
index d2e9150..87d6335 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -54,8 +54,7 @@ IceAllocScratch (
{
if (!iceConn->scratch || size > iceConn->scratch_size)
{
- if (iceConn->scratch)
- free (iceConn->scratch);
+ free (iceConn->scratch);
iceConn->scratch = malloc (size);
iceConn->scratch_size = size;
diff --git a/src/process.c b/src/process.c
index 4100a83..a9a8d08 100644
--- a/src/process.c
+++ b/src/process.c
@@ -1026,8 +1026,7 @@ ProcessConnectionSetup (
iceConn->connection_status = IceConnectRejected;
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
if (iceConn->connection_status == IceConnectRejected)
@@ -1080,8 +1079,7 @@ ProcessConnectionSetup (
if (authData && authDataLen > 0)
free (authData);
- if (errorString)
- free (errorString);
+ free (errorString);
}
if (accept_setup_now)
@@ -1369,8 +1367,7 @@ ProcessAuthReply (
status = IcePaAuthAccepted;
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
if (status != IcePaAuthAccepted)
@@ -1444,8 +1441,7 @@ ProcessAuthReply (
status = IcePaAuthAccepted;
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
if (status == IcePaAuthRejected)
@@ -1559,18 +1555,15 @@ ProcessAuthReply (
_IceErrorSetupFailed (iceConn, ICE_ProtocolSetup,
failureReason);
- if (failureReason)
- free (failureReason);
+ free (failureReason);
}
}
if (free_setup_info)
{
- if (iceConn->protosetup_to_me->his_vendor)
- free (iceConn->protosetup_to_me->his_vendor);
- if (iceConn->protosetup_to_me->his_release)
- free (iceConn->protosetup_to_me->his_release);
+ free (iceConn->protosetup_to_me->his_vendor);
+ free (iceConn->protosetup_to_me->his_release);
free (iceConn->protosetup_to_me);
iceConn->protosetup_to_me = NULL;
}
@@ -1587,8 +1580,8 @@ ProcessAuthReply (
if (authData && authDataLen > 0)
free (authData);
-if (errorString)
- free (errorString);
+
+free (errorString);
IceDisposeCompleteMessage (iceConn, replyData);
return (0);
@@ -2071,8 +2064,7 @@ ProcessProtocolSetup (
ICE_ProtocolSetup, "None of the authentication protocols
specified are supported and host-based authentication failed");
}
- if (hostname)
- free (hostname);
+ free (hostname);
}
}
else
@@ -2118,8 +2110,8 @@ ProcessProtocolSetup (
if (authData && authDataLen > 0)
free (authData);
- if (errorString)
- free (errorString);
+
+ free (errorString);
}
if (accept_setup_now)
@@ -2202,16 +2194,13 @@ ProcessProtocolSetup (
_IceErrorSetupFailed (iceConn, ICE
[PATCH libICE 03/13] save one indentlevel in IceProtocolSetup
save one indentlevel in IceProtocolSetup by early check and
remove a lost free() check
Signed-off-by: Walter Harms
---
src/protosetup.c | 61 +++-
1 file changed, 29 insertions(+), 32 deletions(-)
diff --git a/src/protosetup.c b/src/protosetup.c
index fc6010a..b6aece8 100644
--- a/src/protosetup.c
+++ b/src/protosetup.c
@@ -60,6 +60,7 @@ IceProtocolSetup (
IcePoVersionRec*versionRec = NULL;
intauthCount;
int*authIndices;
+_IceProcessMsgInfo *process_msg_info;
if (errorStringRet && errorLength > 0)
*errorStringRet = '\0';
@@ -235,53 +236,49 @@ IceProtocolSetup (
free (reply.protocol_error.error_message);
}
- if (iceConn->protosetup_to_you->my_auth_indices)
- free (iceConn->protosetup_to_you->my_auth_indices);
+
+ free (iceConn->protosetup_to_you->my_auth_indices);
free (iceConn->protosetup_to_you);
iceConn->protosetup_to_you = NULL;
}
}
-if (accepted)
-{
- _IceProcessMsgInfo *process_msg_info;
+if (!accepted)
+ return (IceProtocolSetupFailure);
- *majorVersionRet = versionRec->major_version;
- *minorVersionRet = versionRec->minor_version;
- *vendorRet = reply.protocol_reply.vendor;
- *releaseRet = reply.protocol_reply.release;
+*majorVersionRet = versionRec->major_version;
+*minorVersionRet = versionRec->minor_version;
+*vendorRet = reply.protocol_reply.vendor;
+*releaseRet = reply.protocol_reply.release;
- /*
-* Increase the reference count for the number of active protocols.
-*/
+/*
+ * Increase the reference count for the number of active protocols.
+ */
- iceConn->proto_ref_count++;
+iceConn->proto_ref_count++;
- /*
-* We may be using a different major opcode for this protocol
-* than the other client. Whenever we get a message, we must
-* map to our own major opcode.
-*/
+/*
+ * We may be using a different major opcode for this protocol
+ * than the other client. Whenever we get a message, we must
+ * map to our own major opcode.
+ */
- hisOpcode = reply.protocol_reply.major_opcode;
+hisOpcode = reply.protocol_reply.major_opcode;
- _IceAddOpcodeMapping (iceConn, hisOpcode, myOpcode);
+_IceAddOpcodeMapping (iceConn, hisOpcode, myOpcode);
- process_msg_info = &iceConn->process_msg_info[hisOpcode -
- iceConn->his_min_opcode];
+process_msg_info = &iceConn->process_msg_info[hisOpcode -
+ iceConn->his_min_opcode];
- process_msg_info->client_data = clientData;
- process_msg_info->accept_flag = 0;
+process_msg_info->client_data = clientData;
+process_msg_info->accept_flag = 0;
- process_msg_info->process_msg_proc.orig_client =
- versionRec->process_msg_proc;
+process_msg_info->process_msg_proc.orig_client =
+ versionRec->process_msg_proc;
+
+return (IceProtocolSetupSuccess);
+
- return (IceProtocolSetupSuccess);
-}
-else
-{
- return (IceProtocolSetupFailure);
-}
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 04/13] check the return of malloc()
check the return of malloc()
Signed-off-by: Walter Harms
---
src/protosetup.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/protosetup.c b/src/protosetup.c
index b6aece8..dbc136e 100644
--- a/src/protosetup.c
+++ b/src/protosetup.c
@@ -110,11 +110,17 @@ IceProtocolSetup (
/*
* Generate the message.
*/
+authCount = 0;
+authIndices = NULL;
if (myProtocol->orig_client->auth_count > 0)
{
authIndices = malloc (
myProtocol->orig_client->auth_count * sizeof (int));
+ if (! authIndices) {
+ strncpy (errorStringRet,"out of memory",errorLength);
+ return (IceProtocolSetupFailure);
+ }
_IceGetPoValidAuthIndices (myProtocol->protocol_name,
iceConn->connection_string,
@@ -123,11 +129,6 @@ IceProtocolSetup (
&authCount, authIndices);
}
-else
-{
- authCount = 0;
- authIndices = NULL;
-}
extra = STRING_BYTES (myProtocol->protocol_name) +
STRING_BYTES (myProtocol->orig_client->vendor) +
@@ -183,6 +184,12 @@ IceProtocolSetup (
replyWait.reply = (IcePointer) &reply;
iceConn->protosetup_to_you = malloc (sizeof (_IceProtoSetupToYouInfo));
+if (! iceConn->protosetup_to_you) {
+ free(authIndices);
+ strncpy (errorStringRet,"out of memory",errorLength);
+ return (IceProtocolSetupFailure);
+}
+
iceConn->protosetup_to_you->my_opcode = myOpcode;
iceConn->protosetup_to_you->my_auth_count = authCount;
iceConn->protosetup_to_you->auth_active = 0;
@@ -199,6 +206,8 @@ IceProtocolSetup (
if (ioErrorOccured)
{
+ free(iceConn->protosetup_to_you);
+ free(authIndices);
strncpy (errorStringRet,
"IO error occured doing Protocol Setup on connection",
errorLength);
@@ -240,6 +249,7 @@ IceProtocolSetup (
free (iceConn->protosetup_to_you->my_auth_indices);
free (iceConn->protosetup_to_you);
iceConn->protosetup_to_you = NULL;
+ free(authIndices);
}
}
@@ -279,6 +289,5 @@ IceProtocolSetup (
versionRec->process_msg_proc;
return (IceProtocolSetupSuccess);
-
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 02/13] make IceProtocolShutdown() more readable
make IceProtocolShutdown() more readable
Signed-off-by: Walter Harms
---
src/shutdown.c | 47 ---
1 file changed, 20 insertions(+), 27 deletions(-)
diff --git a/src/shutdown.c b/src/shutdown.c
index 90e9ded..98376a7 100644
--- a/src/shutdown.c
+++ b/src/shutdown.c
@@ -40,45 +40,38 @@ IceProtocolShutdown (
int majorOpcode
)
{
+int i;
+
if (iceConn->proto_ref_count == 0 || iceConn->process_msg_info == NULL ||
majorOpcode < 1 || majorOpcode > _IceLastMajorOpcode)
{
return (0);
}
-else
-{
- /*
-* Make sure this majorOpcode is really being used.
-*/
-
- int i;
+
+
+/*
+ * Make sure this majorOpcode is really being used.
+ */
- for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
- {
- if (iceConn->process_msg_info[
- i - iceConn->his_min_opcode].in_use &&
-iceConn->process_msg_info[
- i - iceConn->his_min_opcode].my_opcode == majorOpcode)
- break;
- }
+for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
+ {
+ int n=i - iceConn->his_min_opcode;
+ if (iceConn->process_msg_info[n].in_use &&
+ iceConn->process_msg_info[n].my_opcode == majorOpcode)
+ {
- if (i > iceConn->his_max_opcode)
- {
- return (0);
- }
- else
- {
/*
* OK, we can shut down the protocol.
*/
- iceConn->process_msg_info[
- i - iceConn->his_min_opcode].in_use = False;
- iceConn->proto_ref_count--;
+ iceConn->process_msg_info[n].in_use = False;
+ iceConn->proto_ref_count--;
+ return (1);
+ }
+
+ }
- return (1);
- }
-}
+return (0);
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 02/13] make IceProtocolShutdown() more readable
make IceProtocolShutdown() more readable
---
src/shutdown.c | 47 ---
1 file changed, 20 insertions(+), 27 deletions(-)
diff --git a/src/shutdown.c b/src/shutdown.c
index 90e9ded..98376a7 100644
--- a/src/shutdown.c
+++ b/src/shutdown.c
@@ -40,45 +40,38 @@ IceProtocolShutdown (
int majorOpcode
)
{
+int i;
+
if (iceConn->proto_ref_count == 0 || iceConn->process_msg_info == NULL ||
majorOpcode < 1 || majorOpcode > _IceLastMajorOpcode)
{
return (0);
}
-else
-{
- /*
-* Make sure this majorOpcode is really being used.
-*/
-
- int i;
+
+
+/*
+ * Make sure this majorOpcode is really being used.
+ */
- for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
- {
- if (iceConn->process_msg_info[
- i - iceConn->his_min_opcode].in_use &&
-iceConn->process_msg_info[
- i - iceConn->his_min_opcode].my_opcode == majorOpcode)
- break;
- }
+for (i = iceConn->his_min_opcode; i <= iceConn->his_max_opcode; i++)
+ {
+ int n=i - iceConn->his_min_opcode;
+ if (iceConn->process_msg_info[n].in_use &&
+ iceConn->process_msg_info[n].my_opcode == majorOpcode)
+ {
- if (i > iceConn->his_max_opcode)
- {
- return (0);
- }
- else
- {
/*
* OK, we can shut down the protocol.
*/
- iceConn->process_msg_info[
- i - iceConn->his_min_opcode].in_use = False;
- iceConn->proto_ref_count--;
+ iceConn->process_msg_info[n].in_use = False;
+ iceConn->proto_ref_count--;
+ return (1);
+ }
+
+ }
- return (1);
- }
-}
+return (0);
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 05/13] add errormessage and remove unneeded indention
add errormessage and remove unneeded indention
Signed-off-by: Walter Harms
---
src/listen.c | 51 +--
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/src/listen.c b/src/listen.c
index 9a449ae..54aabcd 100644
--- a/src/listen.c
+++ b/src/listen.c
@@ -67,6 +67,8 @@ IceListenForConnections (
for (i = 0; i < transCount; i++)
_IceTransClose (transConns[i]);
free (transConns);
+strncpy (errorStringRet,
+ "Out of memmory", errorLength);
return (0);
}
@@ -186,6 +188,7 @@ IceComposeNetworkIdList (
char *list;
int len = 0;
int i;
+int doneCount = 0;
if (count < 1 || listenObjs == NULL)
return (NULL);
@@ -197,39 +200,35 @@ IceComposeNetworkIdList (
if (list == NULL)
return (NULL);
-else
-{
- int doneCount = 0;
- list[0] = '\0';
+list[0] = '\0';
+for (i = 0; i < count; i++)
+ {
+ if (_IceTransIsLocal (listenObjs[i]->trans_conn))
+ {
+ strcat (list, listenObjs[i]->network_id);
+ doneCount++;
+ if (doneCount < count)
+ strcat (list, ",");
+ }
+ }
+
+if (doneCount < count)
+ {
for (i = 0; i < count; i++)
- {
- if (_IceTransIsLocal (listenObjs[i]->trans_conn))
- {
+ {
+ if (!_IceTransIsLocal (listenObjs[i]->trans_conn))
+ {
strcat (list, listenObjs[i]->network_id);
doneCount++;
if (doneCount < count)
- strcat (list, ",");
- }
- }
+ strcat (list, ",");
+ }
+ }
+ }
- if (doneCount < count)
- {
- for (i = 0; i < count; i++)
- {
- if (!_IceTransIsLocal (listenObjs[i]->trans_conn))
- {
- strcat (list, listenObjs[i]->network_id);
- doneCount++;
- if (doneCount < count)
- strcat (list, ",");
- }
- }
- }
-
- return (list);
-}
+return (list);
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 06/13] check malloc return
check malloc return
failed mallocs will cause segfaults, so add check
also free already allocated memory
Signed-off-by: Walter Harms
---
src/connect.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/connect.c b/src/connect.c
index 276a356..b61449e 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -193,8 +193,8 @@ IceOpenConnection (
iceConn->connect_to_me = NULL;
iceConn->protosetup_to_me = NULL;
-
-if ((iceConn->inbuf = iceConn->inbufptr = malloc (ICE_INBUFSIZE)) == NULL)
+iceConn->inbuf = iceConn->inbufptr = malloc (ICE_INBUFSIZE);
+if ( iceConn->inbuf == NULL)
{
_IceFreeConnection (iceConn);
strncpy (errorStringRet, "Can't malloc", errorLength);
@@ -202,9 +202,10 @@ IceOpenConnection (
}
iceConn->inbufmax = iceConn->inbuf + ICE_INBUFSIZE;
-
-if ((iceConn->outbuf = iceConn->outbufptr = calloc (1, ICE_OUTBUFSIZE)) ==
NULL)
+iceConn->outbuf = iceConn->outbufptr = calloc (1, ICE_OUTBUFSIZE);
+if ( iceConn->outbuf == NULL)
{
+free(iceConn->inbuf);
_IceFreeConnection (iceConn);
strncpy (errorStringRet, "Can't malloc", errorLength);
return (NULL);
@@ -225,6 +226,14 @@ IceOpenConnection (
iceConn->connect_to_you = malloc (sizeof (_IceConnectToYouInfo));
iceConn->connect_to_you->auth_active = 0;
+if (!iceConn->connect_to_you) {
+free(iceConn->outbuf);
+free(iceConn->inbuf);
+ _IceFreeConnection (iceConn);
+ strncpy (errorStringRet, "Can't malloc", errorLength);
+ return (NULL);
+}
+
/*
* Send our byte order.
*/
@@ -467,6 +476,8 @@ ConnectToPeer (char *networkIdsList, char
**actualConnectionRet)
else
{
address = malloc (len + 1);
+ if (!address)
+return (NULL);
address_size = len;
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 07/13] add check for malloc
add check for malloc and a bit untangling
Signed-off-by: Walter Harms
---
src/watch.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/watch.c b/src/watch.c
index abbc265..c60d0c1 100644
--- a/src/watch.c
+++ b/src/watch.c
@@ -48,7 +48,8 @@ IceAddConnectionWatch (
_IceWatchProc *newWatchProc;
inti;
-if ((newWatchProc = malloc (sizeof (_IceWatchProc))) == NULL)
+newWatchProc = malloc (sizeof (_IceWatchProc));
+if ( !newWatchProc )
{
return (0);
}
@@ -75,7 +76,11 @@ IceAddConnectionWatch (
{
_IceWatchedConnection *newWatchedConn =
malloc (sizeof (_IceWatchedConnection));
-
+ if (!newWatchedConn)
+ {
+ IceRemoveConnectionWatch(watchProc,clientData);
+ return (0);
+ }
newWatchedConn->iceConn = _IceConnectionObjs[i];
newWatchedConn->next = NULL;
@@ -86,6 +91,7 @@ IceAddConnectionWatch (
}
return (1);
+
}
@@ -143,6 +149,9 @@ _IceConnectionOpened (
malloc (sizeof (_IceWatchedConnection));
_IceWatchedConnection *watchedConn;
+ if (!newWatchedConn)
+ return ;
+
watchedConn = watchProc->watched_connections;
while (watchedConn && watchedConn->next)
watchedConn = watchedConn->next;
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 08/13] add check for malloc
add check for malloc
fix a potential null pointer deference error and
release already allocated memory
Signed-off-by: Walter Harms
---
src/setauth.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/setauth.c b/src/setauth.c
index 1017b02..1f3e5cc 100644
--- a/src/setauth.c
+++ b/src/setauth.c
@@ -102,6 +102,19 @@ IceSetPaAuthData (
entries[i].auth_data_length;
_IcePaAuthDataEntries[j].auth_data = malloc (
entries[i].auth_data_length);
+ if (! _IcePaAuthDataEntries[j].auth_data)
+ {
+ /*
+ we can not inform the user about a botched update
+ but we can release the memory we already have
+ */
+
+ free(_IcePaAuthDataEntries[j].protocol_name);
+ free(_IcePaAuthDataEntries[j].network_id);
+ free(_IcePaAuthDataEntries[j].auth_name);
+ _IcePaAuthDataEntries[j].auth_data_length = 0;
+ return;
+ }
memcpy (_IcePaAuthDataEntries[j].auth_data,
entries[i].auth_data, entries[i].auth_data_length);
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 09/13] add check for malloc
fix a potential null pointer deference error Signed-off-by: Walter Harms --- src/replywait.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/replywait.c b/src/replywait.c index b25c351..91a8dd6 100644 --- a/src/replywait.c +++ b/src/replywait.c @@ -60,7 +60,8 @@ _IceAddReplyWait ( } savedReplyWait = malloc (sizeof (_IceSavedReplyWait)); - +if (!savedReplyWait) + return; savedReplyWait->reply_wait = replyWait; savedReplyWait->reply_ready = False; savedReplyWait->next = NULL; -- 2.1.4 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 10/13] add check for malloc
From ea066aa04dd118187ca0289053bc4ca5caa0a4a8 Mon Sep 17 00:00:00 2001
fix a potential null pointer deference error
convert malloc() to calloc() to have valid
null pointers on error. so we can release
already allocated memory
Signed-off-by: Walter Harms
---
src/register.c | 53 ++---
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/src/register.c b/src/register.c
index 833714b..2417dd7 100644
--- a/src/register.c
+++ b/src/register.c
@@ -67,7 +67,9 @@ IceRegisterForProtocolSetup (
if (i <= _IceLastMajorOpcode)
{
- p = _IceProtocols[i - 1].orig_client = malloc (sizeof(_IcePoProtocol));
+p = _IceProtocols[i - 1].orig_client = calloc
(1,sizeof(_IcePoProtocol));
+ if (!p)
+ return (-1);
opcodeRet = i;
}
else if (_IceLastMajorOpcode == 255 ||
@@ -82,7 +84,9 @@ IceRegisterForProtocolSetup (
strdup(protocolName);
p = _IceProtocols[_IceLastMajorOpcode].orig_client =
- malloc (sizeof (_IcePoProtocol));
+ calloc (1,sizeof (_IcePoProtocol));
+ if (!p)
+ return (-1);
_IceProtocols[_IceLastMajorOpcode].accept_client = NULL;
@@ -95,15 +99,20 @@ IceRegisterForProtocolSetup (
p->version_count = versionCount;
p->version_recs = malloc (versionCount * sizeof (IcePoVersionRec));
+if (!p->version_recs)
+goto out_of_memory;
+
memcpy (p->version_recs, versionRecs,
versionCount * sizeof (IcePoVersionRec));
if ((p->auth_count = authCount) > 0)
{
p->auth_names = malloc (authCount * sizeof (char *));
-
+ if (!p->auth_names);
+goto out_of_memory;
p->auth_procs = malloc (authCount * sizeof (IcePoAuthProc));
-
+ if (!p->auth_names);
+goto out_of_memory;
for (i = 0; i < authCount; i++)
{
p->auth_names[i] = strdup(authNames[i]);
@@ -119,6 +128,15 @@ IceRegisterForProtocolSetup (
p->io_error_proc = IOErrorProc;
return (opcodeRet);
+
+out_of_memory:
+free(p->auth_procs);
+free(p->auth_names);
+free(p->version_recs);
+free(p->release);
+free(p->vendor);
+free(p);
+return (-1);
}
@@ -163,7 +181,10 @@ IceRegisterForProtocolReply (
if (i <= _IceLastMajorOpcode)
{
p = _IceProtocols[i - 1].accept_client =
- malloc (sizeof (_IcePaProtocol));
+ calloc (1,sizeof (_IcePaProtocol));
+ if (!p)
+ return (-1);
+
opcodeRet = i;
}
else if (_IceLastMajorOpcode == 255 ||
@@ -180,7 +201,9 @@ IceRegisterForProtocolReply (
_IceProtocols[_IceLastMajorOpcode].orig_client = NULL;
p = _IceProtocols[_IceLastMajorOpcode].accept_client =
- malloc (sizeof (_IcePaProtocol));
+ calloc (1,sizeof (_IcePaProtocol));
+ if (!p)
+ return (-1);
opcodeRet = ++_IceLastMajorOpcode;
}
@@ -191,6 +214,9 @@ IceRegisterForProtocolReply (
p->version_count = versionCount;
p->version_recs = malloc (versionCount * sizeof (IcePaVersionRec));
+if (!p->version_recs)
+goto out_of_memory;
+
memcpy (p->version_recs, versionRecs,
versionCount * sizeof (IcePaVersionRec));
@@ -200,8 +226,12 @@ IceRegisterForProtocolReply (
if ((p->auth_count = authCount) > 0)
{
p->auth_names = malloc (authCount * sizeof (char *));
+ if (!p->auth_names);
+goto out_of_memory;
p->auth_procs = malloc (authCount * sizeof (IcePaAuthProc));
+ if (!p->auth_names);
+goto out_of_memory;
for (i = 0; i < authCount; i++)
{
@@ -220,5 +250,14 @@ IceRegisterForProtocolReply (
p->io_error_proc = IOErrorProc;
return (opcodeRet);
-}
+out_of_memory:
+free(p->auth_procs);
+free(p->auth_names);
+free(p->version_recs);
+free(p->release);
+free(p->vendor);
+free(p);
+return (-1);
+
+}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 11/13] add check for mall
fix a potential null pointer deference error and
properly dispose the affected message
Signed-off-by: Walter Harms
---
src/process.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/process.c b/src/process.c
index a9a8d08..89f0403 100644
--- a/src/process.c
+++ b/src/process.c
@@ -923,6 +923,14 @@ ProcessConnectionSetup (
if ((hisAuthCount = message->authCount) > 0)
{
hisAuthNames = malloc (hisAuthCount * sizeof (char *));
+
+ if (!hisAuthNames)
+ {
+ iceConn->connection_status = IceConnectRejected;
+ IceDisposeCompleteMessage (iceConn, pStart);
+ return (0);
+ }
+
EXTRACT_LISTOF_STRING (pData, swap, hisAuthCount, hisAuthNames);
}
@@ -1058,6 +1066,13 @@ ProcessConnectionSetup (
iceConn->connect_to_me = setupInfo =
malloc (sizeof (_IceConnectToMeInfo));
+ if (!iceConn->connect_to_me)
+ {
+ iceConn->connection_status = IceConnectRejected;
+ IceDisposeCompleteMessage (iceConn, pStart);
+ return (0);
+ }
+
setupInfo->my_version_index = myVersionIndex;
setupInfo->his_version_index = hisVersionIndex;
setupInfo->his_vendor = vendor;
@@ -1961,6 +1976,12 @@ ProcessProtocolSetup (
if ((hisAuthCount = message->authCount) > 0)
{
hisAuthNames = malloc (hisAuthCount * sizeof (char *));
+ if (!hisAuthNames)
+ {
+ IceDisposeCompleteMessage (iceConn, pStart);
+ return (0);
+ }
+
EXTRACT_LISTOF_STRING (pData, swap, hisAuthCount, hisAuthNames);
}
@@ -2091,6 +2112,12 @@ ProcessProtocolSetup (
iceConn->protosetup_to_me = setupInfo =
malloc (sizeof (_IceProtoSetupToMeInfo));
+ if (!iceConn->protosetup_to_me)
+ {
+ IceDisposeCompleteMessage (iceConn, pStart);
+ return (0);
+ }
+
setupInfo->his_opcode = hisOpcode;
setupInfo->my_opcode = myOpcode;
setupInfo->my_version_index = myVersionIndex;
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 12/13] add check for malloc
fix a potential null pointer deference error and
IceAllocScratch() do not report size when allocation failes
Signed-off-by: Walter Harms
---
src/misc.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/misc.c b/src/misc.c
index 87d6335..fdc671d 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -57,7 +57,10 @@ IceAllocScratch (
free (iceConn->scratch);
iceConn->scratch = malloc (size);
- iceConn->scratch_size = size;
+ if ( !iceConn->scratch )
+iceConn->scratch_size = 0;
+ else
+iceConn->scratch_size = size;
}
return (iceConn->scratch);
@@ -415,12 +418,14 @@ _IceAddOpcodeMapping (
)
{
if (hisOpcode <= 0 || hisOpcode > 255)
-{
return;
-}
-else if (iceConn->process_msg_info == NULL)
+
+if (iceConn->process_msg_info == NULL)
{
iceConn->process_msg_info = malloc (sizeof (_IceProcessMsgInfo));
+ if ( ! iceConn->process_msg_info )
+ return;
+
iceConn->his_min_opcode = iceConn->his_max_opcode = hisOpcode;
}
else if (hisOpcode < iceConn->his_min_opcode)
@@ -433,6 +438,9 @@ _IceAddOpcodeMapping (
iceConn->process_msg_info = malloc (
newsize * sizeof (_IceProcessMsgInfo));
+ if ( ! iceConn->process_msg_info )
+ return;
+
memcpy (&iceConn->process_msg_info[
iceConn->his_min_opcode - hisOpcode], oldVec,
oldsize * sizeof (_IceProcessMsgInfo));
@@ -460,6 +468,9 @@ _IceAddOpcodeMapping (
iceConn->process_msg_info = malloc (
newsize * sizeof (_IceProcessMsgInfo));
+ if ( ! iceConn->process_msg_info )
+ return;
+
memcpy (iceConn->process_msg_info, oldVec,
oldsize * sizeof (_IceProcessMsgInfo));
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libICE 13/13] make sure buffer is zero filled and report if, allocation failed
make sure buffer is zero filled and
report if allocation failed
Signed-off-by: Walter Harms
---
src/listenwk.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/listenwk.c b/src/listenwk.c
index 7517ea8..bc1da76 100644
--- a/src/listenwk.c
+++ b/src/listenwk.c
@@ -64,11 +64,14 @@ IceListenForWellKnownConnections (
return (0);
}
-if ((listenObjs = malloc (transCount * sizeof (struct _IceListenObj))) ==
NULL)
+listenObjs = calloc (transCount, sizeof (struct _IceListenObj));
+if ( listenObjs == NULL)
{
for (i = 0; i < transCount; i++)
_IceTransClose (transConns[i]);
free (transConns);
+
+strncpy (errorStringRet, "Malloc failed", errorLength);
return (0);
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXau] Avoid out of boundary read access
Am 19.10.2017 22:18, schrieb Tobias Stoeckmann: > If the environment variable HOME is empty, XauFileName triggers an > out of boundary read access (name[1]). true but if HOME="" perhaps we could simply return If HOME consists of a single > character relative path, the output becomes unexpected, because > "HOME=a" leads to "a.Xauthority" instead of "a/.Xauthority". Granted, > a relative HOME path leads to trouble in general, the code should > properly return "a/.Xauthority" nonetheless. Why is that massage for slashDotXauthority done in the first place ? if we drop it: HOME + "/.Xauthority" = ""+ "/.Xauthority" = "/.Xauthority" "a" + "/.Xauthority" = "a/.Xauthority" "a/" + "/.Xauthority" = "a//.Xauthority" a "//" will be condensed to "/" by the system did i miss something ? re, wh > > Signed-off-by: Tobias Stoeckmann > --- > AuFileName.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/AuFileName.c b/AuFileName.c > index 37c8b62..2946c80 100644 > --- a/AuFileName.c > +++ b/AuFileName.c > @@ -85,6 +85,6 @@ XauFileName (void) > bsize = size; > } > snprintf (buf, bsize, "%s%s", name, > - slashDotXauthority + (name[1] == '\0' ? 1 : 0)); > + slashDotXauthority + (name[0] == '/' && name[1] == '\0' ? 1 : > 0)); > return buf; > } ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libXau 0/3] remove redundant null check on calling free()
After the last patch for libXau i checked the code with smatch and started to remove remove redundant null checks. just some cleanup. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libXau 1/3] AuDispose.c:remove redundant null check on calling free()
redundant null check on auth->address calling free()
redundant null check on auth->number calling free()
redundant null check on auth->name calling free()
Signed-off-by: Walter Harms
---
AuDispose.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/AuDispose.c b/AuDispose.c
index 2a9b2f1..355224d 100644
--- a/AuDispose.c
+++ b/AuDispose.c
@@ -34,9 +34,9 @@ void
XauDisposeAuth (Xauth *auth)
{
if (auth) {
- if (auth->address) (void) free (auth->address);
- if (auth->number) (void) free (auth->number);
- if (auth->name) (void) free (auth->name);
+ free (auth->address);
+ free (auth->number);
+ free (auth->name);
if (auth->data) {
(void) bzero (auth->data, auth->data_length);
(void) free (auth->data);
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libXau 3/3] AuRead.c: remove redundant null check on calling free()
this removes simply unneeded code from XauReadAuth
Signed-off-by: Walter Harms
---
AuRead.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/AuRead.c b/AuRead.c
index 5d41f03..d48906b 100644
--- a/AuRead.c
+++ b/AuRead.c
@@ -77,25 +77,25 @@ XauReadAuth (FILE *auth_file)
if (read_counted_string (&local.address_length, &local.address, auth_file)
== 0)
return NULL;
if (read_counted_string (&local.number_length, &local.number, auth_file)
== 0) {
- if (local.address) free (local.address);
+ free (local.address);
return NULL;
}
if (read_counted_string (&local.name_length, &local.name, auth_file) == 0)
{
- if (local.address) free (local.address);
- if (local.number) free (local.number);
+ free (local.address);
+ free (local.number);
return NULL;
}
if (read_counted_string (&local.data_length, &local.data, auth_file) == 0)
{
- if (local.address) free (local.address);
- if (local.number) free (local.number);
- if (local.name) free (local.name);
+ free (local.address);
+ free (local.number);
+ free (local.name);
return NULL;
}
ret = (Xauth *) malloc (sizeof (Xauth));
if (!ret) {
- if (local.address) free (local.address);
- if (local.number) free (local.number);
- if (local.name) free (local.name);
+ free (local.address);
+ free (local.number);
+ free (local.name);
if (local.data) {
bzero (local.data, local.data_length);
free (local.data);
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libXau 2/3]Au FileName.c: remove redundant null check on calling free()
remove redundant null check on calling free()
Signed-off-by: Walter Harms
---
AuFileName.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/AuFileName.c b/AuFileName.c
index 2946c80..4ccda9d 100644
--- a/AuFileName.c
+++ b/AuFileName.c
@@ -68,8 +68,7 @@ XauFileName (void)
}
size = strlen (name) + strlen(&slashDotXauthority[1]) + 2;
if ((size > bsize) || (buf == NULL)) {
- if (buf)
- free (buf);
+ free (buf);
assert(size > 0);
buf = malloc (size);
if (!buf) {
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH libXau 0/3] remove redundant null check on calling free()
Am 28.10.2017 21:18, schrieb Daniel Martin: > Am 28.10.2017 19:16 schrieb "walter harms" : > > After the last patch for libXau i checked the code > with smatch and started to remove remove redundant > > > One redundant "remove" here. ;-) > aehm, yes totally absolutely gone :) re, wh > All 3 patches are > Reviewed-by: Daniel Martin > > > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xfree86: fix gamma compute when palette_size > 256
Am 30.10.2017 07:33, schrieb Qiang Yu:
> palette_(red|green|blue)_size > crtc->gamma_size (=256)
> this may happen when screen has per RGB chanel > 8bit,
> i.e. 30bit depth screen 10bit per RGB.
>
> Signed-off-by: Qiang Yu
> ---
> hw/xfree86/modes/xf86RandR12.c | 96
> ++
> 1 file changed, 69 insertions(+), 27 deletions(-)
>
> diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
> index fe8770d..881c8a6 100644
> --- a/hw/xfree86/modes/xf86RandR12.c
> +++ b/hw/xfree86/modes/xf86RandR12.c
> @@ -1258,40 +1258,82 @@ xf86RandR12CrtcComputeGamma(xf86CrtcPtr crtc, LOCO
> *palette,
>
> for (shift = 0; (gamma_size << shift) < (1 << 16); shift++);
>
> -gamma_slots = crtc->gamma_size / palette_red_size;
> -for (i = 0; i < palette_red_size; i++) {
> -value = palette[i].red;
> -if (gamma_red)
> -value = gamma_red[value];
> -else
> -value <<= shift;
> +if (crtc->gamma_size >= palette_red_size) {
minor nitpick:
IMHO: if you use if (crtc->gamma_size < palette_red_size) and switch the
blocks it is more easy to read.
And since the if () are a bit large, did you consider to make it into
3 separate functions to improve readability ?
re,
wh
> +gamma_slots = crtc->gamma_size / palette_red_size;
> +for (i = 0; i < palette_red_size; i++) {
> +value = palette[i].red;
> +if (gamma_red)
> +value = gamma_red[value];
> +else
> +value <<= shift;
>
> -for (j = 0; j < gamma_slots; j++)
> -crtc->gamma_red[i * gamma_slots + j] = value;
> +for (j = 0; j < gamma_slots; j++)
> +crtc->gamma_red[i * gamma_slots + j] = value;
> +}
> }
> +else {
> +gamma_slots = palette_red_size / crtc->gamma_size;
> +for (i = 0; i < crtc->gamma_size; i++) {
> +value = palette[i * gamma_slots].red;
> +if (gamma_red)
> +value = gamma_red[value];
> +else
> +value <<= shift;
>
> -gamma_slots = crtc->gamma_size / palette_green_size;
> -for (i = 0; i < palette_green_size; i++) {
> -value = palette[i].green;
> -if (gamma_green)
> -value = gamma_green[value];
> -else
> -value <<= shift;
> +crtc->gamma_red[i] = value;
> +}
> +}
> +
> +if (crtc->gamma_size >= palette_green_size) {
> +gamma_slots = crtc->gamma_size / palette_green_size;
> +for (i = 0; i < palette_green_size; i++) {
> +value = palette[i].green;
> +if (gamma_green)
> +value = gamma_green[value];
> +else
> +value <<= shift;
> +
> +for (j = 0; j < gamma_slots; j++)
> +crtc->gamma_green[i * gamma_slots + j] = value;
> +}
> +}
> +else {
> +gamma_slots = palette_green_size / crtc->gamma_size;
> +for (i = 0; i < crtc->gamma_size; i++) {
> +value = palette[i * gamma_slots].green;
> +if (gamma_green)
> +value = gamma_green[value];
> +else
> +value <<= shift;
>
> -for (j = 0; j < gamma_slots; j++)
> -crtc->gamma_green[i * gamma_slots + j] = value;
> +crtc->gamma_green[i] = value;
> +}
> }
>
> -gamma_slots = crtc->gamma_size / palette_blue_size;
> -for (i = 0; i < palette_blue_size; i++) {
> -value = palette[i].blue;
> -if (gamma_blue)
> -value = gamma_blue[value];
> -else
> -value <<= shift;
> +if (crtc->gamma_size >= palette_blue_size) {
> +gamma_slots = crtc->gamma_size / palette_blue_size;
> +for (i = 0; i < palette_blue_size; i++) {
> +value = palette[i].blue;
> +if (gamma_blue)
> +value = gamma_blue[value];
> +else
> +value <<= shift;
>
> -for (j = 0; j < gamma_slots; j++)
> -crtc->gamma_blue[i * gamma_slots + j] = value;
> +for (j = 0; j < gamma_slots; j++)
> +crtc->gamma_blue[i * gamma_slots + j] = value;
> +}
> +}
> +else {
> +gamma_slots = palette_blue_size / crtc->gamma_size;
> +for (i = 0; i < crtc->gamma_size; i++) {
> +value = palette[i * gamma_slots].blue;
> +if (gamma_blue)
> +value = gamma_blue[value];
> +else
> +value <<= shift;
> +
> +crtc->gamma_blue[i] = value;
> +}
> }
> }
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: Xstartup handle DISPLAY=(null) correctly
at what circumstances can DISPLAY == '(null)' ? looks more like a bug in an Xserver. re, wh Am 20.11.2017 06:40, schrieb xiaoguang wang: > > > > ___ > xorg-devel@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] os: Fix strtok/free crash in ComputeLocalClient
Am 06.12.2017 12:16, schrieb Tomasz Śniatowski: > Don't reuse cmd for strtok output to ensure the proper pointer is > freed afterwards. > > The code incorrectly assumed the pointer returned by strtok(cmd, ":") > would always point to cmd. However, strtok(str, sep) != str if str > begins with sep. This caused an invalid-free crash when running > a program under X with a name beginning with a colon. > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=104123 > Yes, strtok returns a pointer to its arguments. btw: strtok is not thread-safe, could that be an issue with that function ? re, wh > Signed-off-by: Tomasz Śniatowski > --- > os/access.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/os/access.c b/os/access.c > index 8828e0834..97246160c 100644 > --- a/os/access.c > +++ b/os/access.c > @@ -1137,12 +1137,12 @@ ComputeLocalClient(ClientPtr client) > /* Cut off any colon and whatever comes after it, see > * > https://lists.freedesktop.org/archives/xorg-devel/2015-December/048164.html > */ > -cmd = strtok(cmd, ":"); > +char *tok = strtok(cmd, ":"); > > #if !defined(WIN32) || defined(__CYGWIN__) > -ret = strcmp(basename(cmd), "ssh") != 0; > +ret = strcmp(basename(tok), "ssh") != 0; > #else > -ret = strcmp(cmd, "ssh") != 0; > +ret = strcmp(tok, "ssh") != 0; > #endif > > free(cmd); ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xf86-video-vesa 3/4] Use VBEFreeVBEInfo not free
Am 31.01.2018 16:48, schrieb Adam Jackson: > A VbeInfoBlock has substructure, just freeing the object will leak. > Unfortunately VBEFreeVBEInfo does not check for NULL first so we have > to. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=25029 > Signed-off-by: Adam Jackson > --- > src/vesa.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/src/vesa.c b/src/vesa.c > index 3f5b81c..2d3c10d 100644 > --- a/src/vesa.c > +++ b/src/vesa.c > @@ -596,7 +596,8 @@ VESAFreeRec(ScrnInfoPtr pScrn) > } > #endif > free(pVesa->monitor); > -free(pVesa->vbeInfo); > +if (pVesa->vbeInfo) > + VBEFreeVBEInfo(pVesa->vbeInfo); since all free function accept NULL these days it may be more efficient to teach VBEFreeVBEInfo to accept NULL. just my 2 cents, re, wh > free(pVesa->pal); > free(pVesa->savedPal); > free(pVesa->fonts); ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
Am 05.02.2018 02:47, schrieb Giuseppe Bilotta:
> We used to accept something like --scale 2x3junk as a valid input
> (scaling x by 2 and y by 3), even though this isn't really a valid
> scaling factor.
>
> Fix by making sure there is nothing after the parsed number(s).
>
> Signed-off-by: Giuseppe Bilotta
> ---
> xrandr.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xrandr.c b/xrandr.c
> index 1085a95..6a38cf2 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -3014,11 +3014,12 @@ main (int argc, char **argv)
> if (!strcmp ("--scale", argv[i]))
> {
> double sx, sy;
> + char junk;
> if (!config_output) argerr ("%s must be used after --output\n",
> argv[i]);
> if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2)
> + if (sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk) != 2)
> {
> - if (sscanf (argv[i], "%lf", &sx) != 1)
> + if (sscanf (argv[i], "%lf%c", &sx, &junk) != 1)
> argerr ("failed to parse '%s' as a scaling factor\n",
> argv[i]);
> sy = sx;
> }
can the scanf be converted to strtod ? there you get an endpointer by default.
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xrandr v4 2/5] xrandr: stricter --scale argument parsing
Am 05.02.2018 11:24, schrieb Giuseppe Bilotta:
> On Mon, Feb 5, 2018 at 10:44 AM, walter harms wrote:
>>
>> Am 05.02.2018 02:47, schrieb Giuseppe Bilotta:
>>> {
>>> double sx, sy;
>>> + char junk;
>>> if (!config_output) argerr ("%s must be used after --output\n",
>>> argv[i]);
>>> if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
>>> - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2)
>>> + if (sscanf (argv[i], "%lfx%lf%c", &sx, &sy, &junk) != 2)
>>> {
>>> - if (sscanf (argv[i], "%lf", &sx) != 1)
>>> + if (sscanf (argv[i], "%lf%c", &sx, &junk) != 1)
>>> argerr ("failed to parse '%s' as a scaling factor\n",
>>> argv[i]);
>>> sy = sx;
>>> }
>>
>> can the scanf be converted to strtod ? there you get an endpointer by
>> default.
>
> I'm not a big fan of strtod because with it it's impossible to know if
> a conversion actually happened. xrandr --scale ' ' would actually be
> accepted (resulting in a scale value of 0), while the scanf catches
> it. For the same reason I use two sscanf instead of a single one
> (because that wouldn't be able to catch something like xrandr --scale
> 1j).
>
> Of course there's also to be said that we could reject a scale factor
> of 0, regardless of whether it comes from a correct parsing of the
> string '0.0' or from the parse of an empty string (but of course then
> we couldn't customize the error message to differentiate between
> “incorrect parse” and “value out of range”).
>
Ok, i understand. My guess is that a scale factor of 0 would be rejected
or interessting things will happen
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xorgproto] randr: MONITORINFO has outputs, not crtcs
It would be nice to have an explanation why this change. re, wh Am 06.02.2018 02:03, schrieb Giuseppe Bilotta: > --- > randrproto.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/randrproto.txt b/randrproto.txt > index c06bc90..f57301d 100644 > --- a/randrproto.txt > +++ b/randrproto.txt > @@ -2363,14 +2363,14 @@ A.1.1 Common Types added in version 1.5 of the > protocol > 4 ATOMname > 1 BOOLprimary > 1 BOOLautomatic > - 2 CARD16 ncrtcs > + 2 CARD16 noutputs > 2 INT16 x > 2 INT16 y > 2 CARD16 width in pixels > 2 CARD16 height in pixels > 4 CARD32 width in millimeters > 4 CARD32 height in millimeters > - 4*n CRTCcrtcs > + 4*n OUTPUT outputs > └─── > > A.2 Protocol Requests ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Do nothing when there are no platform devices
Am 09.09.2015 14:58, schrieb Marcin Juszkiewicz:
> Probably not best solution yet but closer.
>
> commit dd1aa1f2fb9d91d766e020ac5c2cf474ce951788
> Author: Marcin Juszkiewicz
> Date: Wed Sep 9 14:53:51 2015 +0200
>
> xserver: ignore boot_vga on AArch64
>
> AArch64 machines firmware does not initialize graphics output so
> boot_vga is not set.
>
> Without this patch Xserver needs to be told where gfx card is by
> providing BusId entry in xorg.conf file.
>
> Signed-off-by: Marcin Juszkiewicz
>
> diff --git a/hw/xfree86/common/xf86platformBus.c
> b/hw/xfree86/common/xf86platformBus.c
> index f1e9423..9dd56ec 100644
> --- a/hw/xfree86/common/xf86platformBus.c
> +++ b/hw/xfree86/common/xf86platformBus.c
> @@ -136,10 +136,14 @@ platform_find_pci_info(struct xf86_platform_device *pd,
> char *busid)
> if (info) {
> pd->pdev = info;
> pci_device_probe(info);
> +#ifndef __aarch64__
> if (pci_device_is_boot_vga(info)) {
> +#endif
> primaryBus.type = BUS_PLATFORM;
> primaryBus.id.plat = pd;
> +#ifndef __aarch64__
> }
> +#endif
I do not now if the patch is working oe not but to reduce the #if forrest
#ifndef __aarch64_
if (pci_device_is_boot_vga(info))
#endif
{
primaryBus.type = BUS_PLATFORM;
primaryBus.id.plat = pd;
}
more over i would change the architecture depending part into something
more functional like IGNORE_BOOT_VGA
note i am not the maintainer, just my 2 cents
re,
wh
> }primaryBus.id.plat = pd;
> pci_iterator_destroy(iter);
>
> ___
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xquartz 03/10] xdmauth: Correct miscall of abs() to instrad call labs()
some nitpicking ...
man 3 difftime
i pretty sure it will cause a fundamental change ...
just my 2 cents
re,
wh
Am 15.10.2015 00:13, schrieb Jeremy Huddleston Sequoia:
> xdmauth.c:230:13: warning: absolute value function 'abs' given an argument of
> type 'long' but has parameter of
> type
> 'int'
> which may cause truncation of value [-Wabsolute-value,Semantic Issue]
> if (abs(now - client->time) > TwentyFiveMinutes) {
> ^
> xdmauth.c:230:13: note: use function 'labs' instead [Semantic Issue]
> if (abs(now - client->time) > TwentyFiveMinutes) {
> ^~~
> labs
> xdmauth.c:302:9: warning: absolute value function 'abs' given an argument of
> type 'long' but has parameter of type
> 'int' which
> may cause truncation of value [-Wabsolute-value,Semantic Issue]
> if (abs(client->time - now) > TwentyMinutes) {
> ^
> xdmauth.c:302:9: note: use function 'labs' instead [Semantic Issue]
> if (abs(client->time - now) > TwentyMinutes) {
> ^~~
> labs
>
> Signed-off-by: Jeremy Huddleston Sequoia
> ---
> os/xdmauth.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/os/xdmauth.c b/os/xdmauth.c
> index f11cbb9..482bc67 100644
> --- a/os/xdmauth.c
> +++ b/os/xdmauth.c
> @@ -227,7 +227,7 @@ XdmClientAuthTimeout(long now)
> prev = 0;
> for (client = xdmClients; client; client = next) {
> next = client->next;
> -if (abs(now - client->time) > TwentyFiveMinutes) {
> +if (labs(now - client->time) > TwentyFiveMinutes) {
> if (prev)
> prev->next = next;
> else
> @@ -299,7 +299,7 @@ XdmAuthorizationValidate(unsigned char *plain, int length,
> }
> now += clockOffset;
> XdmClientAuthTimeout(now);
> -if (abs(client->time - now) > TwentyMinutes) {
> +if (labs(client->time - now) > TwentyMinutes) {
> free(client);
> if (reason)
> *reason = "Excessive XDM-AUTHORIZATION-1 time offset";
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xquartz 04/10] randr: Silence -Wshift-negative-value warnings
Am 15.10.2015 00:33, schrieb Alan Coopersmith: > On 10/14/15 03:13 PM, Jeremy Huddleston Sequoia wrote: >> rrtransform.c:199:23: warning: shifting a negative signed value is >> undefined [-Wshift-negative-value,Semantic Issue] >> rot_cos = F(-1); > > >> -rot_cos = F(-1); >> +rot_cos = F(~0u); > > Is -1 guaranteed to be ~0u on all platforms? Or just all the ones we know > and care about? > > Would it be better to instead do: > -#define F(x)IntToxFixed(x) > +#define F(x)IntToxFixed((uint32_t) x) > > in randr/rrtransform.c ? > i always feel bad when seeing 1char macros. Perhaps we can remove it also by simple S&R ? just my 2 cents. re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Also dump passive grabs on XF86LogGrabInfo
Am 15.10.2015 18:32, schrieb Michael Stapelberg:
> This patch is not entirely finished, I’ve marked the places that need further
> attention with TODO. Any input on these, and on the patch in general, is
> appreciated.
>
> Listing passive grabs is useful for debugging the case where users don’t know
> why a keybinding is not consumed by a particular application (typically
> because
> another application is grabbing the binding).
>
> I’ve tried to be consistent in the messages with other places that dump grabs
> or clients.
>
> ---
> dix/window.c| 108
>
> hw/xfree86/dixmods/xkbPrivate.c | 2 +
> include/window.h| 1 +
> 3 files changed, 111 insertions(+)
>
> diff --git a/dix/window.c b/dix/window.c
> index d57f320..5e15a72 100644
> --- a/dix/window.c
> +++ b/dix/window.c
> @@ -127,6 +127,7 @@ Equipment Corporation.
> #include "compint.h"
> #endif
> #include "selection.h"
> +#include "inpututils.h"
>
> #include "privates.h"
> #include "xace.h"
> @@ -273,6 +274,113 @@ log_window_info(WindowPtr pWin, int depth)
> ErrorF("\n");
> }
>
> +static void
> +log_grab_info(void *value, XID id, void *cdata)
> +{
> +GrabPtr pGrab;
> +int i, j;
> +pGrab = (GrabPtr)value;
> +
> +ErrorF(" grab 0x%lx (%s), type '%s' on window 0x%lx\n",
> + (unsigned long) pGrab->resource,
> + (pGrab->grabtype == XI2) ? "xi2" :
> + ((pGrab->grabtype == CORE) ? "core" : "xi1"),
> + pGrab->type == ButtonPress ? "ButtonPress" :
> + // TODO: where do DeviceButtonPress and DeviceKeyPress come from?
> + // They are undefined when compiling.
> + //pGrab->type == DeviceButtonPress ? "DeviceButtonPress" :
> + pGrab->type == XI_ButtonPress ? "XI_ButtonPress" :
> + pGrab->type == KeyPress ? "KeyPress" :
> + //pGrab->type == DeviceKeyPress ? "DeviceKeyPress" :
> + "XI_KeyPress",
> + (unsigned long) pGrab->window->drawable.id,
> + pGrab->device->id);
The mix of comments and long lines make it hard to read.
I would suggest to make this a function like:
static char *grab2txt(pGrab->grabtype)
{
switch (pGrab->grabtype)
{
case XI2: return "xi2" :
case CORE: return "core" ;
}
return "xi1";
}
(does not compile i know)
same for pGrab->type
re,
wh
> +ErrorF("detail %d (mask %lu), modifiersDetail %d (mask %lu)\n",
> + pGrab->detail.exact,
> + pGrab->detail.pMask ? (unsigned long) *(pGrab->detail.pMask) : 0,
> + pGrab->modifiersDetail.exact,
> + pGrab->modifiersDetail.pMask ?
> + (unsigned long) *(pGrab->modifiersDetail.pMask) :
> + (unsigned long) 0);
> +ErrorF("device '%s' (%d), modifierDevice '%s' (%d)\n",
> + pGrab->device->name, pGrab->device->id,
> + pGrab->modifierDevice->name, pGrab->modifierDevice->id);
> +if (pGrab->grabtype == CORE) {
> +ErrorF("core event mask 0x%lx\n",
> + (unsigned long) pGrab->eventMask);
> +}
> +else if (pGrab->grabtype == XI) {
> +ErrorF("xi1 event mask 0x%lx\n",
> + (unsigned long) pGrab->eventMask);
> +}
> +else if (pGrab->grabtype == XI2) {
> +for (i = 0; i < xi2mask_num_masks(pGrab->xi2mask); i++) {
> +const unsigned char *mask;
> +int print;
> +
> +print = 0;
> +for (j = 0; j < XI2MASKSIZE; j++) {
> +mask = xi2mask_get_one_mask(pGrab->xi2mask, i);
> +if (mask[j]) {
> +print = 1;
> +break;
> +}
> +}
> +if (!print)
> +continue;
> +ErrorF(" xi2 event mask 0x");
> +for (j = 0; j < xi2mask_mask_size(pGrab->xi2mask); j++)
> +ErrorF("%x", mask[j]);
> +ErrorF("\n");
> +}
> +}
> +ErrorF("owner-events %s, kb %d ptr %d, confine 0x%lx, cursor
> 0x%lx\n",
> + pGrab->ownerEvents ? "true" : "false",
> + pGrab->keyboardMode, pGrab->pointerMode,
> + pGrab->confineTo ? (unsigned long) pGrab->confineTo->drawable.id
> : 0,
> + pGrab->cursor ? (unsigned long) pGrab->cursor->id : 0);
> +
> +// TODO: dump another grab if there is pGrab->next
> +}
> +
> +void
> +PrintPassiveGrabs(void)
> +{
> +int i;
> +LocalClientCredRec *lcc;
> +pid_t clientpid;
> +const char *cmdname;
> +const char *cmdargs;
> +
> +ErrorF("Printing all currently registered grabs\n");
> +
> +for (i = 1; i < currentMaxClients; i++) {
> +if (!clients[i] || clients[i]->clientState != ClientStateRunning)
> +continue;
> +
> +clientpid = GetClientPid(clients[i]);
> +cmdname = GetClientCmdName(clients[i]);
> +cmdargs = GetClientCmdAr
Re: [PATCH] glamor: fix crash when drawing nothing
Am 16.10.2015 18:11, schrieb Rob Clark: > For example, in the PolyFillRect() path w/ nrect==0, we end up in > glamor_get_vbo_space(size=0): > >(gdb) bt >#0 0x007fb73df340 in raise () from /lib64/libc.so.6 >#1 0x007fb73e0fb8 in abort () from /lib64/libc.so.6 >#2 0x007fb73d84f4 in __assert_fail_base () from /lib64/libc.so.6 >#3 0x007fb73d859c in __assert_fail () from /lib64/libc.so.6 >#4 0x0045dc38 in glamor_get_vbo_space (screen=0x8dcb60, > size=size@entry=0, vbo_offset=0x7ff250, vbo_offset@entry=0x7ff2d0) at > glamor_vbo.c:112 >#5 0x004541ac in glamor_poly_fill_rect_gl (prect=0x11be650, > nrect=0, gc=0x4cda70 , drawable=0x7ff388) at > glamor_rects.c:72 >#6 glamor_poly_fill_rect (drawable=0x7ff388, gc=0x4cda70 > , nrect=0, prect=0x11be650) at glamor_rects.c:162 >#7 0x00557d0c in damagePolyFillRect (pDrawable=0x112c300, > pGC=0xe81580, nRects=0, pRects=) at damage.c:1194 >#8 0x004cdb20 in miPaintWindow (pWin=, > prgn=0x7ff388, what=) at miexpose.c:540 >#9 0x004db260 in miClearToBackground (pWin=0x112c300, > x=, y=, w=, h=, > generateExposures=0) at miwindow.c:116 >#10 0x004646e4 in ProcClearToBackground (client=0x111e810) at > dispatch.c:1592 >#11 0x0046907c in Dispatch () at dispatch.c:430 >#12 0x0046cf90 in dix_main (argc=5, argv=0x7ff628, > envp=) at main.c:300 >#13 0x007fb73cb68c in __libc_start_main () from /lib64/libc.so.6 >#14 0x0042b3e8 in _start () > > Also fixed a bunch of other call-sites which could in theory trigger the > same issue. > > v2: add back the early return if size==0 in glamor_get_vbo_space() just > in case. We'd hit a GL error in glamor_put_vbo_space() if we ever ended > up with a call path that hit that (so keep the earlier early-returns), > but a GL error is better than a crash so keep this extra safety-net. > > Signed-off-by: Rob Clark > --- > glamor/glamor_copy.c | 3 +++ > glamor/glamor_dash.c | 3 +++ > glamor/glamor_glyphblt.c | 3 +++ > glamor/glamor_points.c | 3 +++ > glamor/glamor_rects.c| 3 +++ > glamor/glamor_render.c | 3 +++ > glamor/glamor_segs.c | 3 +++ > glamor/glamor_spans.c| 3 +++ > glamor/glamor_text.c | 3 +++ > glamor/glamor_vbo.c | 3 +++ > 10 files changed, 30 insertions(+) > > diff --git a/glamor/glamor_copy.c b/glamor/glamor_copy.c > index 028acf2..97db20c 100644 > --- a/glamor/glamor_copy.c > +++ b/glamor/glamor_copy.c > @@ -317,6 +317,9 @@ glamor_copy_fbo_fbo_draw(DrawablePtr src, > const glamor_facet *copy_facet; > int n; > > +if (nbox == 0) > +return TRUE; > + > glamor_make_current(glamor_priv); > > if (gc && !glamor_set_planemask(gc->depth, gc->planemask)) > diff --git a/glamor/glamor_dash.c b/glamor/glamor_dash.c > index 101228e..b961951 100644 > --- a/glamor/glamor_dash.c > +++ b/glamor/glamor_dash.c > @@ -328,6 +328,9 @@ glamor_poly_segment_dash_gl(DrawablePtr drawable, GCPtr > gc, > int add_last; > int i; > > +if (nseg == 0) > +return TRUE; > + > if (!(prog = glamor_dash_setup(drawable, gc))) > return FALSE; > > diff --git a/glamor/glamor_glyphblt.c b/glamor/glamor_glyphblt.c > index 1791f6d..0cd3406 100644 > --- a/glamor/glamor_glyphblt.c > +++ b/glamor/glamor_glyphblt.c > @@ -175,6 +175,9 @@ glamor_push_pixels_gl(GCPtr gc, PixmapPtr bitmap, > INT16 *points = NULL; > char *vbo_offset; > > +if ((w * h) == 0) > +return TRUE; > + ( ) seems a bit superfluous. re, wh > if (w * h > MAXINT / (2 * sizeof(float))) > goto bail; > > diff --git a/glamor/glamor_points.c b/glamor/glamor_points.c > index 3ba4a69..e0aa87e 100644 > --- a/glamor/glamor_points.c > +++ b/glamor/glamor_points.c > @@ -48,6 +48,9 @@ glamor_poly_point_gl(DrawablePtr drawable, GCPtr gc, int > mode, int npt, DDXPoint > char *vbo_offset; > int box_x, box_y; > > +if (npt == 0) > +return TRUE; > + > pixmap_priv = glamor_get_pixmap_private(pixmap); > if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) > goto bail; > diff --git a/glamor/glamor_rects.c b/glamor/glamor_rects.c > index c378e4a..26d1401 100644 > --- a/glamor/glamor_rects.c > +++ b/glamor/glamor_rects.c > @@ -53,6 +53,9 @@ glamor_poly_fill_rect_gl(DrawablePtr drawable, > char *vbo_offset; > int box_x, box_y; > > +if (nrect == 0) > +return TRUE; > + > pixmap_priv = glamor_get_pixmap_private(pixmap); > if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)) > goto bail; > diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c > index c3a8f17..3ad4507 100644 > --- a/glamor/glamor_render.c > +++ b/glamor/glamor_render.c > @@ -621,6 +621,9 @@ glamor_setup_composite_vbo(ScreenPtr screen, int n_verts) > > vert_size = n_verts * glamor_priv->vb_stride; > > +if (vert_size == 0) > +
Re: [patch xdm 2/4] Remove SCO, Unixware, OS/2 support
Looks good to me.
while you are fixing the ifdef forrest
the is a line with:
# ifndef QNX4
IMHO that should read HAVE_INITGROUPS
re,
wh
Am 02.12.2015 12:22, schrieb Matthieu Herrb:
> Signed-off-by: Matthieu Herrb
> ---
> config/Xsession.cpp | 18 --
> greeter/verify.c| 97
> +
> include/dm.h| 8 ++---
> xdm/dm.c| 7
> xdm/resource.c | 8 ++---
> xdm/session.c | 78 +++---
> 6 files changed, 17 insertions(+), 199 deletions(-)
>
> diff --git config/Xsession.cpp config/Xsession.cpp
> index 36ccbd0..aed2ab7 100644
> --- config/Xsession.cpp
> +++ config/Xsession.cpp
> @@ -62,23 +62,5 @@ else
> if [ -r "$resources" ]; then
> BINDIR/xrdb -load "$resources"
> fi
> -#if defined(__SCO__) || defined(__UNIXWARE__)
> -[ -r /etc/default/xdesktops ] && {
> -. /etc/default/xdesktops
> -}
> -
> -[ -r /etc/default/xdm ] && {
> -. /etc/default/xdm
> -}
> -
> -XCOMM Allow the user to over-ride the system default desktop
> -[ -r $HOME/.xdmdesktop ] && {
> -. $HOME/.xdmdesktop
> -}
> -
> -[ -n "$XDESKTOP" ] && {
> -exec `eval $XDESKTOP`
> -}
> -#endif
> exec BINDIR/xsm
> fi
> diff --git greeter/verify.c greeter/verify.c
> index b009e2b..cf1f47b 100644
> --- greeter/verify.c
> +++ greeter/verify.c
> @@ -49,9 +49,6 @@ from The Open Group.
> # include
> # include
> # include
> -#elif defined(USESECUREWARE)
> -# include
> -# include
> #endif
>
> #include "greet.h"
> @@ -226,99 +223,7 @@ Verify (struct display *d, struct greet_info *greet,
> struct verify_info *verify)
> break;
> }
> }
> -#elif defined(USESECUREWARE) /* !USE_BSDAUTH */
> -/*
> - * This is a global variable and will be referenced in at least session.c
> - */
> -struct smp_user_info *userp = 0;
> -
> -_X_INTERNAL
> -int
> -Verify (struct display *d, struct greet_info *greet, struct verify_info
> *verify)
> -{
> - int ret, pwtries = 0, nis, delay;
> - char *reason = 0;
> - struct passwd *p;
> - char *shell, *home, **argv;
> -
> - Debug ("Verify %s ...\n", greet->name);
> -
> - p = getpwnam (greet->name);
> - endpwent();
> -
> - if (!p || strlen (greet->name) == 0) {
> -LogError ("getpwnam() failed.\n");
> -bzero(greet->password, strlen(greet->password));
> -return 0;
> - }
> -
> - ret = smp_check_user (SMP_LOGIN, greet->name, 0, 0, &userp, &pwtries,
> -&reason, &nis, &delay);
> - if (ret != SMP_RETIRED && userp->retired)
> -ret = userp->result = SMP_RETIRED;
> - Debug ("smp_check_user returns %d\n", ret);
> -
> - switch (ret) {
> -case SMP_FAIL:
> - Debug ("Out of memory in smp_check_user\n");
> - goto smp_fail;
> -case SMP_EXTFAIL:
> - Debug ("SMP_EXTFAIL: %s", reason);
> - goto smp_fail;
> -case SMP_NOTAUTH:
> - Debug ("Not authorized\n");
> - goto smp_fail;
> -case SMP_TERMLOCK:
> - Debug ("Terminal is locked!\n");
> - goto smp_fail;
> -case SMP_ACCTLOCK:
> - Debug ("Account is locked\n");
> - goto smp_fail;
> -case SMP_RETIRED:
> - Debug ("Account is retired\n");
> - goto smp_fail;
> -case SMP_OVERRIDE:
> - Debug ("On override device ... proceeding\n");
> - break;
> -case SMP_NULLPW:
> - Debug ("NULL password entry\n");
> - if (!greet->allow_null_passwd) {
> -goto smp_fail;
> - }
> - break;
> -case SMP_BADUSER:
> - Debug ("User not found in protected password database\n");
> - goto smp_fail;
> -case SMP_PWREQ:
> - Debug ("Password change required\n");
> - goto smp_fail;
> -case SMP_HASPW:
> - break;
> -default:
> - Debug ("Unhandled smp_check_user return %d\n", ret);
> -smp_fail:
> - sleep(delay);
> - smp_audit_fail (userp, 0);
> - bzero(greet->password, strlen(greet->password));
> - return 0;
> - break;
> - }
> -
> - if (ret != SMP_NULLPW) {
> -/*
> - * If we require a password, check it.
> - */
> -ret = smp_check_pw (greet->password, userp, &reason);
> -switch (ret) {
> - case SMP_CANCHANGE:
> - case SMP_CANTCHANGE:
> - case SMP_OVERRIDE:
> -break;
> - default:
> -goto smp_fail;
> -}
> - }
> -#else /* !USE_BSDAUTH && !USESECUREWARE */
> +#else /* !USE_BSDAUTH */
> _X_INTERNAL
> int
> Verify (struct display *d, struct greet_info *greet, struct verify_info
> *verify)
> diff --git include/dm.h include/dm.h
> index 13e7839..9116d6f 100644
> --- include/dm.h
> +++ include/dm.h
> @@ -78,10 +78,6 @@ from The Open Group.
> # include
> # else
> # define _POSIX_SOURCE
> -# ifdef __SCO__
> -#include
> -#include
> -# endif
> # include
> # undef _POSIX_SO
Re: [PATCH:libX11] Bug 93184: read_EncodingInfo invalid free
Am 04.12.2015 08:30, schrieb Alan Coopersmith:
> Free the correct bits of memory if we run out and need to unwind
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93184
> Signed-off-by: Alan Coopersmith
> ---
> modules/om/generic/omGeneric.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/modules/om/generic/omGeneric.c b/modules/om/generic/omGeneric.c
> index 610361b..724f47e 100644
> --- a/modules/om/generic/omGeneric.c
> +++ b/modules/om/generic/omGeneric.c
> @@ -1877,13 +1877,13 @@ read_EncodingInfo(
> {
> FontData font_data,ret;
> char *buf, *bufptr,*scp;
> -int len;
> +int len, i;
> font_data = Xcalloc(count, sizeof(FontDataRec));
> if (font_data == NULL)
> return NULL;
>
> ret = font_data;
> -for ( ; count-- > 0; font_data++) {
> +for (i = 0; i < count; i++, font_data++) {
> /*
> strcpy(buf, *value++);
> */
> @@ -1895,7 +1895,8 @@ read_EncodingInfo(
> len = strlen(buf);
> font_data->name = Xmalloc(len + 1);
> if (font_data->name == NULL) {
> -Xfree(font_data);
> +free_fontdataOM(ret, i + 1);
> +Xfree(ret);
> return NULL;
> }
> strncpy(font_data->name, buf,len);
could you also replace strlen+malloc+strncopy with strdup() ?
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:libX11] Bug 93184: read_EncodingInfo invalid free
Am 04.12.2015 09:40, schrieb Alan Coopersmith:
> On 12/ 4/15 12:05 AM, walter harms wrote:
>> Am 04.12.2015 08:30, schrieb Alan Coopersmith:
>>> @@ -1895,7 +1895,8 @@ read_EncodingInfo(
>>> len = strlen(buf);
>>> font_data->name = Xmalloc(len + 1);
>>> if (font_data->name == NULL) {
>>> -Xfree(font_data);
>>> +free_fontdataOM(ret, i + 1);
>>> +Xfree(ret);
>>> return NULL;
>>> }
>>> strncpy(font_data->name, buf,len);
>>
>>
>> could you also replace strlen+malloc+strncopy with strdup() ?
>
> Not in this case without rewriting the code paths more - the missing
> context here is:
>
> if ((bufptr = strchr(buf, ':'))) {
> len = (int)(bufptr - buf);
> bufptr++ ;
> } else
> len = strlen(buf);
> font_data->name = Xmalloc(len + 1);
> if (font_data->name == NULL) {
> free_fontdataOM(ret, i + 1);
> Xfree(ret);
> return NULL;
> }
> strncpy(font_data->name, buf,len);
> font_data->name[len] = 0;
>
> Maybe could do something like
> if (... strchr ...)
> font_data->name = strndup
> else
> font_data->name = strdup
>
> but that sounds like something to keep to a separate patch.
>
yes, NTL i suggest a FIXME comment here :)
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] os: Treat ssh as a non-local client
Am 08.12.2015 19:55, schrieb Adam Jackson:
> By the time we get to ComputeLocalClient, we've already done
> NextAvailableClient → ReserveClientIds → DetermineClientCmd (assuming
> we're built with #define CLIENTIDS), so we can look up the name of the
> client process and refuse to treat ssh's X forwarding as if it were
> local.
>
> Signed-off-by: Adam Jackson
> ---
> os/access.c | 19 ---
> 1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/os/access.c b/os/access.c
> index 10a48c3..f9619ae 100644
> --- a/os/access.c
> +++ b/os/access.c
> @@ -1081,9 +1081,8 @@ ResetHosts(const char *display)
> }
> }
>
> -/* Is client on the local host */
> -Bool
> -ComputeLocalClient(ClientPtr client)
> +static Bool
> +xtransLocalClient(ClientPtr client)
> {
> int alen, family, notused;
> Xtransaddr *from = NULL;
> @@ -1116,6 +1115,20 @@ ComputeLocalClient(ClientPtr client)
> return FALSE;
> }
>
unfortunately, that means if my client is called
"the_exceptional_secure_shell" it would break, right ?
there i would ask, could that be made on option ?
(default value e.g. ssh) other programms may benefit
also.
re,
wh
> +/* Is client on the local host */
> +Bool
> +ComputeLocalClient(ClientPtr client)
> +{
> +if (!xtransLocalClient(client))
> +return FALSE;
> +
> +if (client->clientIds)
> +if (!strncmp(client->clientIds->cmdname, "ssh", 3))
> +return FALSE;
> +
> +return TRUE;
> +}
> +
> /*
> * Return the uid and all gids of a connected local client
> * Allocates a LocalClientCredRec - caller must call FreeLocalClientCreds
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xf86-input-libinput] Drain the fd after opening
Am 16.12.2015 01:15, schrieb Peter Hutterer: > Make sure we discard any events that may have been enqueued before we > re-opened the fd. Specifically, if we're using systemd-logind the fd remains > open when we disable/enable the device, allowing events to queue up on the > fd. These events are then replayed once the device is re-opened. > > This only applies to a device being disabled via the protocol, on VT switch > logind closes the fd and we don't see enqueued events. > > Signed-off-by: Peter Hutterer > --- > src/xf86libinput.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/src/xf86libinput.c b/src/xf86libinput.c > index ee2165a..2fde9ed 100644 > --- a/src/xf86libinput.c > +++ b/src/xf86libinput.c > @@ -1175,6 +1175,8 @@ open_restricted(const char *path, int flags, void *data) > } > > fd = xf86OpenSerial(pInfo->options); > + xf86FlushInput(fd); > + > return fd < 0 ? -errno : fd; > } > can xf86FlushInput() handle fd<0 ? perhaps it would be better to make clear that this is an error case like: fd = xf86OpenSerial(pInfo->options); if (fd<0) return -errno ; xf86FlushInput(fd); return fd; just my 2 cents, re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Contribute.
Am 19.12.2015 01:00, schrieb Edi Anderson: > Hi guys, I'm a programmer and would like to contribute to the project but > do not know where to start. Could anyone help me ?. > > Hi and welcome to the project, to make thinks clear i am also a beginner, i wanted to start last year but i got stuck when i run out ot time, so i consider my self a beginner here. You can contribute in all areas. The only question is: How good is your programming skill ? What is your background knowledge ? What means the certain areas are harder to start with than others. Personally i say: Contributing to documentation is a good starting point. Writing examples, updating man-pages and friends. It will help to learn how things are done and are a good introduction to the code. If you are a experienced programmer than you can look at patches posted on the list and send some comment (bonuspoints when you can check the patch). Most people like to hear everything is fine but providing an alternative is at least some reason to discuss. An other way is to look at the bugtracker, just checking what is still a bug is a lot of work, and give you some in-side-view of the code. ntl, an other way to participate is to answer questions on the mailing list :) re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH] Allows holding down two keys to repeat both ("Two-key repeat")
Am 29.02.2016 23:59, schrieb thomassross:
> From: Germain Bossu
>
> https://git.framasoft.org/bobo/xkb_repeat/blob/master/patches/xkb-repeat2-easy.patch
> ---
> include/xkbsrv.h | 1 +
> xkb/xkbAccessX.c | 9 +
> 2 files changed, 10 insertions(+)
>
> diff --git a/include/xkbsrv.h b/include/xkbsrv.h
> index cc6307a..5d8d913 100644
> --- a/include/xkbsrv.h
> +++ b/include/xkbsrv.h
> @@ -173,6 +173,7 @@ typedef struct _XkbSrvInfo {
> KeyCode slowKey;
> KeyCode slowKeyEnableKey;
> KeyCode repeatKey;
> +KeyCode repeatKey2;
> CARD8 krgTimerActive;
> CARD8 beepType;
> CARD8 beepCount;
> diff --git a/xkb/xkbAccessX.c b/xkb/xkbAccessX.c
> index 02e820b..e0a135c 100644
> --- a/xkb/xkbAccessX.c
> +++ b/xkb/xkbAccessX.c
> @@ -80,6 +80,7 @@ AccessXInit(DeviceIntPtr keybd)
> xkbi->inactiveKey = 0;
> xkbi->slowKey = 0;
> xkbi->repeatKey = 0;
> +xkbi->repeatKey2 = 0;
> xkbi->krgTimerActive = _OFF_TIMER;
> xkbi->beepType = _BEEP_NONE;
> xkbi->beepCount = 0;
> @@ -318,6 +319,8 @@ AccessXRepeatKeyExpire(OsTimerPtr timer, CARD32 now, void
> *arg)
> return 0;
>
> AccessXKeyboardEvent(dev, ET_KeyPress, xkbi->repeatKey, TRUE);
> +if (xkbi->repeatKey2 != 0)
> +AccessXKeyboardEvent(dev, ET_KeyPress, xkbi->repeatKey2, TRUE);
>
> return xkbi->desc->ctrls->repeat_interval;
> }
> @@ -327,6 +330,7 @@ AccessXCancelRepeatKey(XkbSrvInfoPtr xkbi, KeyCode key)
> {
> if (xkbi->repeatKey == key)
> xkbi->repeatKey = 0;
> +xkbi->repeatKey2 = 0;
> return;
> }
this looks strange, either the indent is wrong or same braces are missing ?
if (xkbi->repeatKey != key)
return
xkbi->repeatKey = 0;
xkbi->repeatKey2 = 0;
below there is the same pattern, maybe you want to use AccessXCancelRepeatKey()
there ?
just my 2 cents,
re,
wh
>
> @@ -541,6 +545,10 @@ AccessXFilterPressEvent(DeviceEvent *event, DeviceIntPtr
> keybd)
> DebugF("Starting software autorepeat...\n");
> if (xkbi->repeatKey == key)
> ignoreKeyEvent = TRUE;
> +else if (xkbi->repeatKey != 0)
> +xkbi->repeatKey2 = key;
> +else if (xkbi->repeatKey2 == key)
> +ignoreKeyEvent = TRUE;
> else {
> xkbi->repeatKey = key;
> xkbi->repeatKeyTimer = TimerSet(xkbi->repeatKeyTimer,
> @@ -644,6 +652,7 @@ AccessXFilterReleaseEvent(DeviceEvent *event,
> DeviceIntPtr keybd)
> */
> if (xkbi->repeatKey == key) {
> xkbi->repeatKey = 0;
> +xkbi->repeatKey2 = 0;
> }
>
> if ((ctrls->enabled_ctrls & XkbAccessXTimeoutMask) &&
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver 1/9] xvmc: Fix unchecked AddResource
Am 07.03.2016 23:20, schrieb Julien Cristau:
> Signed-off-by: Julien Cristau
> ---
> Xext/xvmc.c | 17 ++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/Xext/xvmc.c b/Xext/xvmc.c
> index 64eda92..7565c17 100644
> --- a/Xext/xvmc.c
> +++ b/Xext/xvmc.c
> @@ -253,6 +253,10 @@ ProcXvMCCreateContext(ClientPtr client)
> free(pContext);
> return result;
> }
> +if (!AddResource(pContext->context_id, XvMCRTContext, pContext)) {
> +free(data);
> +return BadAlloc;
> +}
>
> rep = (xvmcCreateContextReply) {
> .type = X_Reply,
> @@ -266,7 +270,6 @@ ProcXvMCCreateContext(ClientPtr client)
> WriteToClient(client, sizeof(xvmcCreateContextReply), &rep);
> if (dwords)
> WriteToClient(client, dwords << 2, data);
> -AddResource(pContext->context_id, XvMCRTContext, pContext);
>
> free(data);
>
> @@ -329,6 +332,11 @@ ProcXvMCCreateSurface(ClientPtr client)
> free(pSurface);
> return result;
> }
> +if (!AddResource(pSurface->surface_id, XvMCRTSurface, pSurface)) {
> +free(data);
> +return BadAlloc;
> +}
> +
I do not see the whole picture but the free a few lines before indicate that
the error path may need to free other previously allocated resources.
re,
wh
> rep = (xvmcCreateSurfaceReply) {
> .type = X_Reply,
> .sequenceNumber = client->sequence,
> @@ -338,7 +346,6 @@ ProcXvMCCreateSurface(ClientPtr client)
> WriteToClient(client, sizeof(xvmcCreateSurfaceReply), &rep);
> if (dwords)
> WriteToClient(client, dwords << 2, data);
> -AddResource(pSurface->surface_id, XvMCRTSurface, pSurface);
>
> free(data);
>
> @@ -445,6 +452,11 @@ ProcXvMCCreateSubpicture(ClientPtr client)
> free(pSubpicture);
> return result;
> }
> +if (!AddResource(pSubpicture->subpicture_id, XvMCRTSubpicture,
> pSubpicture)) {
> +free(data);
> +return BadAlloc;
> +}
> +
> rep = (xvmcCreateSubpictureReply) {
> .type = X_Reply,
> .sequenceNumber = client->sequence,
> @@ -462,7 +474,6 @@ ProcXvMCCreateSubpicture(ClientPtr client)
> WriteToClient(client, sizeof(xvmcCreateSubpictureReply), &rep);
> if (dwords)
> WriteToClient(client, dwords << 2, data);
> -AddResource(pSubpicture->subpicture_id, XvMCRTSubpicture, pSubpicture);
>
> free(data);
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:makedepend] Use do { ... } while(0) idiom to make debug() macro safer
Am 12.03.2016 19:15, schrieb Alan Coopersmith:
> Cleans up several -Wempty-body warnings from gcc 5.3
>
> Signed-off-by: Alan Coopersmith
> ---
> def.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/def.h b/def.h
> index 1930cde..59670a9 100644
> --- a/def.h
> +++ b/def.h
> @@ -82,9 +82,9 @@ extern int _debugmask;
> * 3 show #include SYMBOL
> * 4-6 unused
> */
> -#define debug(level,arg) { if (_debugmask & (1 << level)) warning arg; }
> +#define debug(level,arg) do { if (_debugmask & (1 << level)) warning arg; }
> while(0)
> #else
> -#define debug(level,arg) /**/
> +#define debug(level,arg) do { /**/ } while (0)
> #endif /* DEBUG */
>
> typedef unsigned char boolean;
#definedebug(level,arg) while(0)
should be sufficient (not tested)
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH:makedepend] Use do { ... } while(0) idiom to make debug() macro safer
Am 12.03.2016 19:43, schrieb Matt Turner:
> On Sat, Mar 12, 2016 at 10:24 AM, walter harms wrote:
>>
>>
>> Am 12.03.2016 19:15, schrieb Alan Coopersmith:
>>> Cleans up several -Wempty-body warnings from gcc 5.3
>>>
>>> Signed-off-by: Alan Coopersmith
>>> ---
>>> def.h | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/def.h b/def.h
>>> index 1930cde..59670a9 100644
>>> --- a/def.h
>>> +++ b/def.h
>>> @@ -82,9 +82,9 @@ extern int _debugmask;
>>> * 3 show #include SYMBOL
>>> * 4-6 unused
>>> */
>>> -#define debug(level,arg) { if (_debugmask & (1 << level)) warning arg; }
>>> +#define debug(level,arg) do { if (_debugmask & (1 << level)) warning arg;
>>> } while(0)
>>> #else
>>> -#define debug(level,arg) /**/
>>> +#define debug(level,arg) do { /**/ } while (0)
>>> #endif /* DEBUG */
>>>
>>> typedef unsigned char boolean;
>>
>>
>> #definedebug(level,arg) while(0)
>>
>> should be sufficient (not tested)
>
> Maybe, but that sure looks strange to my eye, while the do { ... }
> while(0) pattern is well recognized.
for me other way around :)
never mind, both are a noop, someone could propose this as C extension ,)
re,
wh
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libX11 1/4] fix shadow warning
Signed-off-by: walter harms
---
resolve anoying shadow warning from gcc
---
src/StrKeysym.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/StrKeysym.c b/src/StrKeysym.c
index 12fce68..be77a93 100644
--- a/src/StrKeysym.c
+++ b/src/StrKeysym.c
@@ -115,7 +115,7 @@ XStringToKeysym(_Xconst char *s)
{
XrmValue result;
XrmRepresentation from_type;
- char c;
+ char d;
XrmQuark names[2];
names[0] = _XrmInternalStringToQuark(s, p - s - 1, sig, False);
@@ -126,10 +126,10 @@ XStringToKeysym(_Xconst char *s)
val = 0;
for (i = 0; i < result.size - 1; i++)
{
- c = ((char *)result.addr)[i];
- if ('0' <= c && c <= '9') val = (val<<4)+c-'0';
- else if ('a' <= c && c <= 'f') val = (val<<4)+c-'a'+10;
- else if ('A' <= c && c <= 'F') val = (val<<4)+c-'A'+10;
+ d = ((char *)result.addr)[i];
+ if ('0' <= d && d <= '9') val = (val<<4)+d-'0';
+ else if ('a' <= d && d <= 'f') val = (val<<4)+d-'a'+10;
+ else if ('A' <= d && d <= 'F') val = (val<<4)+d-'A'+10;
else return NoSymbol;
}
return val;
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libX11 2/4] _XIOError(dpy); will never return so remore dead
Signed-off-by: walter harms
---
remove dead code as _XIOError will never return
---
src/xcb_io.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/src/xcb_io.c b/src/xcb_io.c
index bd26a62..4826621 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -700,10 +700,8 @@ Status _XReply(Display *dpy, xReply *rep, int extra, Bool
discard)
/* it's not an error, but we don't have a reply, so it's an I/O
* error. */
if(!reply)
- {
_XIOError(dpy);
- return 0;
- }
+
/* there's no error and we have a reply. */
dpy->xcb->reply_data = reply;
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libX11 3/4] remove argument check for free() adjust one inden
Signed-off-by: walter harms
---
there is no need to check the argument for free()
---
modules/im/ximcp/imDefIc.c | 57 +++---
1 file changed, 28 insertions(+), 29 deletions(-)
diff --git a/modules/im/ximcp/imDefIc.c b/modules/im/ximcp/imDefIc.c
index 143bd59..7564dba 100644
--- a/modules/im/ximcp/imDefIc.c
+++ b/modules/im/ximcp/imDefIc.c
@@ -231,10 +231,9 @@ _XimReCreateIC(ic)
_XimRegisterFilter(ic);
MARK_IC_CONNECTED(ic);
-if (save_ic->private.proto.ic_resources)
- Xfree(save_ic->private.proto.ic_resources);
-if (save_ic->private.proto.ic_inner_resources)
- Xfree(save_ic->private.proto.ic_inner_resources);
+
+Xfree(save_ic->private.proto.ic_resources);
+Xfree(save_ic->private.proto.ic_inner_resources);
Xfree(save_ic);
return True;
@@ -833,7 +832,7 @@ _XimDestroyICCheck(
&& (imid == im->private.proto.imid)
&& (buf_s[2] & XIM_ICID_VALID)
&& (icid == ic->private.proto.icid))
-ret = False;
+ret = False;
return ret;
}
@@ -845,22 +844,22 @@ _XimProtoICFree(
Xim im = (Xim)ic->core.im;
#endif
-if (ic->private.proto.preedit_font) {
- Xfree(ic->private.proto.preedit_font);
- ic->private.proto.preedit_font = NULL;
-}
-if (ic->private.proto.status_font) {
- Xfree(ic->private.proto.status_font);
- ic->private.proto.status_font = NULL;
-}
+
+Xfree(ic->private.proto.preedit_font);
+ic->private.proto.preedit_font = NULL;
+
+
+Xfree(ic->private.proto.status_font);
+ic->private.proto.status_font = NULL;
+
if (ic->private.proto.commit_info) {
_XimFreeCommitInfo(ic);
ic->private.proto.commit_info = NULL;
}
-if (ic->private.proto.ic_inner_resources) {
- Xfree(ic->private.proto.ic_inner_resources);
- ic->private.proto.ic_inner_resources = NULL;
-}
+
+Xfree(ic->private.proto.ic_inner_resources);
+ic->private.proto.ic_inner_resources = NULL;
+
#ifdef XIM_CONNECTABLE
if (IS_SERVER_CONNECTED(im) && IS_RECONNECTABLE(im)) {
@@ -868,18 +867,18 @@ _XimProtoICFree(
}
#endif /* XIM_CONNECTABLE */
-if (ic->private.proto.saved_icvalues) {
- Xfree(ic->private.proto.saved_icvalues);
- ic->private.proto.saved_icvalues = NULL;
-}
-if (ic->private.proto.ic_resources) {
- Xfree(ic->private.proto.ic_resources);
- ic->private.proto.ic_resources = NULL;
-}
-if (ic->core.hotkey) {
- Xfree(ic->core.hotkey);
- ic->core.hotkey = NULL;
-}
+
+Xfree(ic->private.proto.saved_icvalues);
+ic->private.proto.saved_icvalues = NULL;
+
+
+Xfree(ic->private.proto.ic_resources);
+ic->private.proto.ic_resources = NULL;
+
+
+Xfree(ic->core.hotkey);
+ic->core.hotkey = NULL;
+
return;
}
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libX11 4/4] fix shadow char_size
Signed-off-by: walter harms
---
fix shadow warning by renaming the var
---
src/xlibi18n/lcCT.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/xlibi18n/lcCT.c b/src/xlibi18n/lcCT.c
index b161300..1f413e9 100644
--- a/src/xlibi18n/lcCT.c
+++ b/src/xlibi18n/lcCT.c
@@ -1021,19 +1021,19 @@ cstoct(
) {
while (csstr_len > 0 && ct_len > 0) {
unsigned char ch = * (const unsigned char *) csptr;
-int char_size = (ch < 0xc0 ? 1 :
+int ch_size = (ch < 0xc0 ? 1 :
ch < 0xe0 ? 2 :
ch < 0xf0 ? 3 :
ch < 0xf8 ? 4 :
ch < 0xfc ? 5 :
6);
int i;
-if (!(csstr_len >= char_size && ct_len >= char_size))
+if (!(csstr_len >= ch_size && ct_len >= ch_size))
break;
-for (i = char_size; i > 0; i--)
+for (i = ch_size; i > 0; i--)
*ctptr++ = *csptr++;
-csstr_len -= char_size;
-ct_len -= char_size;
+csstr_len -= ch_size;
+ct_len -= ch_size;
}
} else {
while (csstr_len > 0 && ct_len > 0) {
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libX11 0/4] various fixes
hi list, This fixes a bunch of bugs inside libX11. I did some experiments with smatch https://blogs.oracle.com/linuxkernel/entry/smatch_static_analysis_tool_overview It found some more but all of them are fixed easily. These patches will not change the function of the current code In case of problems you can find all patched here also: https://github.com/xtforever/libX11/tree/master/src re, wh ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2 3/9] xfree86: Create VT atoms from the root window callback (v2)
Am 05.04.2016 19:54, schrieb Adam Jackson:
> v2: Fix swapped callback args
>
> Signed-off-by: Adam Jackson
> ---
> hw/xfree86/common/xf86Init.c | 67
> +---
> 1 file changed, 25 insertions(+), 42 deletions(-)
>
> diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c
> index f5f407e..744af7c 100644
> --- a/hw/xfree86/common/xf86Init.c
> +++ b/hw/xfree86/common/xf86Init.c
> @@ -381,10 +381,28 @@ AddSeatId(CallbackListPtr *pcbl, void *data, void
> *screen)
> "Failed to register seat property\n");
> }
>
> -/* The memory storing the initial value of the XFree86_has_VT root window
> - * property. This has to remain available until server start-up, so we just
> - * use a global. */
> -static CARD32 HasVTValue = 1;
> +static void
> +AddVTAtoms(CallbackListPtr *pcbl, void *data, void *screen)
> +{
> +#define VT_ATOM_NAME "XFree86_VT"
Just for my curiosity is VT_ATOM_NAME used somewhere else ?
Otherwise a s/VT_ATOM_NAME/"XFree86_VT" would be sufficient
just my 2 cents,
re,
wh
> +int err, HasVT = 1;
> +ScreenPtr pScreen = screen;
> +Atom VTAtom = MakeAtom(VT_ATOM_NAME, sizeof(VT_ATOM_NAME) - 1, TRUE);
> +Atom HasVTAtom = MakeAtom(HAS_VT_ATOM_NAME, sizeof(HAS_VT_ATOM_NAME) - 1,
> + TRUE);
> +
> +err = dixChangeWindowProperty(serverClient, pScreen->root, VTAtom,
> + XA_INTEGER, 32, PropModeReplace, 1,
> + &xf86Info.vtno, FALSE);
> +
> +err |= dixChangeWindowProperty(serverClient, pScreen->root, HasVTAtom,
> + XA_INTEGER, 32, PropModeReplace, 1,
> + &HasVT, FALSE);
> +
> +if (err != Success)
> +xf86DrvMsg(pScreen->myNum, X_WARNING,
> + "Failed to register VT properties\n");
> +}
>
> /*
> * InitOutput --
> @@ -727,44 +745,6 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char
> **argv)
> }
> formatsDone = TRUE;
>
> -if (xf86Info.vtno >= 0) {
> -#define VT_ATOM_NAME "XFree86_VT"
> -Atom VTAtom = -1;
> -Atom HasVTAtom = -1;
> -CARD32 *VT = NULL;
> -CARD32 *HasVT = &HasVTValue;
> -int ret;
> -
> -/* This memory needs to stay available until the screen has been
> - initialized, and we can create the property for real.
> - */
> -if ((VT = malloc(sizeof(CARD32))) == NULL) {
> -FatalError
> -("Unable to make VT property - out of memory.
> Exiting...\n");
> -}
> -*VT = xf86Info.vtno;
> -
> -VTAtom = MakeAtom(VT_ATOM_NAME, sizeof(VT_ATOM_NAME) - 1, TRUE);
> -HasVTAtom = MakeAtom(HAS_VT_ATOM_NAME,
> - sizeof(HAS_VT_ATOM_NAME) - 1, TRUE);
> -
> -for (i = 0, ret = Success; i < xf86NumScreens && ret == Success;
> - i++) {
> -ret =
> -xf86RegisterRootWindowProperty(xf86Screens[i]->scrnIndex,
> - VTAtom, XA_INTEGER, 32, 1,
> - VT);
> -if (ret == Success)
> -ret = xf86RegisterRootWindowProperty(xf86Screens[i]
> - ->scrnIndex,
> - HasVTAtom,
> XA_INTEGER,
> - 32, 1, HasVT);
> -if (ret != Success)
> -xf86DrvMsg(xf86Screens[i]->scrnIndex, X_WARNING,
> - "Failed to register VT properties\n");
> -}
> -}
> -
> /* If a screen uses depth 24, show what the pixmap format is */
> for (i = 0; i < xf86NumScreens; i++) {
> if (xf86Screens[i]->depth == 24) {
> @@ -798,6 +778,9 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char
> **argv)
> xf86EnableIO();
> }
>
> +if (xf86Info.vtno >= 0)
> +AddCallback(&RootWindowFinalizeCallback, AddVTAtoms, NULL);
> +
> if (SeatId)
> AddCallback(&RootWindowFinalizeCallback, AddSeatId, SeatId);
>
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2 6/9] xfree86: Make xf86SetDDCproperties work more than once per generation
Am 05.04.2016 19:54, schrieb Adam Jackson:
> We can call this more than once via xf86OutputSetEDID since hotplug is
> actually a thing in RANDR 1.2, but xf86RegisterRootWindowProperty merely
> adds the data to a list to be applied to the root at CreateWindow time,
> so calls past the first (for a given screen) would have no effect until
> server regen.
>
> Once we've initialised pScrn->pScreen is filled in, so we can just set
> the property directly.
>
> Signed-off-by: Adam Jackson
> ---
> hw/xfree86/ddc/ddcProperty.c | 34 --
> 1 file changed, 28 insertions(+), 6 deletions(-)
>
> diff --git a/hw/xfree86/ddc/ddcProperty.c b/hw/xfree86/ddc/ddcProperty.c
> index c51c3e6..a31e9c7 100644
> --- a/hw/xfree86/ddc/ddcProperty.c
> +++ b/hw/xfree86/ddc/ddcProperty.c
> @@ -27,6 +27,7 @@
>
> #include "xf86.h"
> #include "xf86DDC.h"
> +#include "xf86Priv.h"
> #include
> #include "property.h"
> #include "propertyst.h"
> @@ -34,17 +35,38 @@
>
> #define EDID1_ATOM_NAME "XFree86_DDC_EDID1_RAWDATA"
>
> +static int
> +edidSize(const xf86MonPtr DDC)
> +{
> +if (DDC->ver.version != 1)
> +return 0;
> +
> + return 128 +
> + (DDC->flags & EDID_COMPLETE_RAWDATA ? DDC->no_sections * 128 : 0);
> +}
> +
i would that a bit more readable, like
if (DDC->flags & EDID_COMPLETE_RAWDATA)
return 128+DDC->no_sections * 128;
return 128;
> static void
> -edidMakeAtom(int i, const char *name, CARD8 *data, int size)
> +setRootWindowEDID(ScreenPtr pScreen, xf86MonPtr DDC)
> +{
> +Atom atom = MakeAtom(EDID1_ATOM_NAME, strlen(EDID1_ATOM_NAME), TRUE);
> +
> +dixChangeWindowProperty(serverClient, pScreen->root, atom, XA_INTEGER,
> +8, PropModeReplace, edidSize(DDC), DDC->rawData,
> +FALSE);
> +}
> +
> +static void
> +edidMakeAtom(int i, const char *name, xf86MonPtr DDC)
> {
> Atom atom;
> unsigned char *atom_data;
> +int size = edidSize(DDC);
>
> if (!(atom_data = malloc(size * sizeof(CARD8
> return;
>
> atom = MakeAtom(name, strlen(name), TRUE);
> -memcpy(atom_data, data, size);
> +memcpy(atom_data, DDC->rawData, size);
> xf86RegisterRootWindowProperty(i, atom, XA_INTEGER, 8, size, atom_data);
> }
>
> @@ -54,10 +76,10 @@ addRootWindowProperties(ScrnInfoPtr pScrn, xf86MonPtr DDC)
> int scrnIndex = pScrn->scrnIndex;
>
> if (DDC->ver.version == 1) {
> -int size = 128 +
> -(DDC->flags & EDID_COMPLETE_RAWDATA ? DDC->no_sections * 128 :
> 0);
> -
> -edidMakeAtom(scrnIndex, EDID1_ATOM_NAME, DDC->rawData, size);
> +if (xf86Initialising)
> +edidMakeAtom(scrnIndex, EDID1_ATOM_NAME, DDC);
> +else
> +setRootWindowEDID(pScrn->pScreen, DDC);
> }
> else {
> xf86DrvMsg(scrnIndex, X_PROBED, "unexpected EDID version %d.%d\n",
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2 7/9] xfree86: Remove a never-hit diagnostic message
edidSize() checks for if (DDC->ver.version != 1) (see 6/9)
so this is bogus either ?
re,
wh
Am 05.04.2016 19:54, schrieb Adam Jackson:
> Practically speaking, the EDID major version is never not 1.
>
> Signed-off-by: Adam Jackson
> ---
> hw/xfree86/ddc/ddcProperty.c | 15 ---
> 1 file changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/hw/xfree86/ddc/ddcProperty.c b/hw/xfree86/ddc/ddcProperty.c
> index a31e9c7..66e7ba8 100644
> --- a/hw/xfree86/ddc/ddcProperty.c
> +++ b/hw/xfree86/ddc/ddcProperty.c
> @@ -75,17 +75,10 @@ addRootWindowProperties(ScrnInfoPtr pScrn, xf86MonPtr DDC)
> {
> int scrnIndex = pScrn->scrnIndex;
>
> -if (DDC->ver.version == 1) {
> -if (xf86Initialising)
> -edidMakeAtom(scrnIndex, EDID1_ATOM_NAME, DDC);
> -else
> -setRootWindowEDID(pScrn->pScreen, DDC);
> -}
> -else {
> -xf86DrvMsg(scrnIndex, X_PROBED, "unexpected EDID version %d.%d\n",
> - DDC->ver.version, DDC->ver.revision);
> -return;
> -}
> +if (xf86Initialising)
> +edidMakeAtom(scrnIndex, EDID1_ATOM_NAME, DDC);
> +else
> +setRootWindowEDID(pScrn->pScreen, DDC);
> }
>
> Bool
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH libX11 1/3] fix more shadow warning
Signed-off-by: walter harms
---
fix an other shadow warning by renaming the var
---
modules/im/ximcp/imCallbk.c | 6 +++---
modules/im/ximcp/imLcLkup.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/modules/im/ximcp/imCallbk.c b/modules/im/ximcp/imCallbk.c
index 4e091d8..ead0806 100644
--- a/modules/im/ximcp/imCallbk.c
+++ b/modules/im/ximcp/imCallbk.c
@@ -624,16 +624,16 @@ _XimPreeditCaretCallback(Xim im,
*/
{
CARD8 buf[sz_ximPacketHeader + sz_ximPreeditCaretReply];
- INT16 len = sz_XIMID + sz_XICID + sz_ximPreeditCaretReply;
+ INT16 rlen = sz_XIMID + sz_XICID + sz_ximPreeditCaretReply;
int p;
- _XimSetHeader((XPointer)buf, XIM_PREEDIT_CARET_REPLY, 0, &len);
+ _XimSetHeader((XPointer)buf, XIM_PREEDIT_CARET_REPLY, 0, &rlen);
p = XIM_HEADER_SIZE;
*(CARD16*)&buf[p] = (CARD16)im->private.proto.imid; p += sz_CARD16;
*(CARD16*)&buf[p] = (CARD16)ic->private.proto.icid; p += sz_CARD16;
*(CARD32*)&buf[p] = (CARD32)cbs.position;
- if (!(_XimWriteData(im, len, buf))) {
+ if (!(_XimWriteData(im, rlen, buf))) {
return XimCbError;
}
_XimFlushData(im);
diff --git a/modules/im/ximcp/imLcLkup.c b/modules/im/ximcp/imLcLkup.c
index 9e4aec3..56dba96 100644
--- a/modules/im/ximcp/imLcLkup.c
+++ b/modules/im/ximcp/imLcLkup.c
@@ -61,8 +61,8 @@ _XimLocalMbLookupString(XIC xic, XKeyEvent *ev, char *buffer,
int bytes,
||(ic->private.local.brl_committed != 0))) {
if (ic->private.local.brl_committed != 0) { /* Braille Event */
unsigned char pattern = ic->private.local.brl_committed;
- char mb[XLC_PUBLIC(ic->core.im->core.lcd, mb_cur_max)];
- ret = _Xlcwctomb(ic->core.im->core.lcd, mb, BRL_UC_ROW | pattern);
+ char mb2[XLC_PUBLIC(ic->core.im->core.lcd, mb_cur_max)];
+ ret = _Xlcwctomb(ic->core.im->core.lcd, mb2, BRL_UC_ROW | pattern);
if(ret > bytes) {
if(status) *status = XBufferOverflow;
return(ret);
@@ -74,7 +74,7 @@ _XimLocalMbLookupString(XIC xic, XKeyEvent *ev, char *buffer,
int bytes,
} else {
if(status) *status = XLookupChars;
}
- memcpy(buffer, mb, ret);
+ memcpy(buffer, mb2, ret);
} else {
if(keysym) {
if(status) *status = XLookupKeySym;
--
2.1.4
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
