Re: [RFC] DeepColor Visual Class Extension

2017-10-18 Thread Alex Goins
On Sun, 15 Oct 2017, Keith Packard wrote:

> Alex Goins  writes:
> 
> > Thanks, Adam.
> >
> > Here's an updated version of the spec:
> 
> This is looking very good. I don't have any architectural concerns at
> this point, just some editorial comments.

Thanks, that's good to hear.

> > Rendering to DeepColor windows using the core protocol, however, is loosely
> > defined.
> 
> It seems to be actually fairly well defined to me. If core pixel values
> could be 'round-tripped' through the deep storage, then core rendering
> would be exact. That seems pretty simple with the UINT* formats when
> used with a non-linear colorspace -- just use an identity mapping.

Core rendering would be exact relative to core operations e.g. GetImage, but
relative to the HDR format it would be more loosely defined. Probably just a
matter of wording.

Using the identity mapping for UINT* formats would work, as long as we don't
care about an accurate representation of the SDR content when interpreted by HDR
consumers. With a more accurate transfer function, SDR content could look
correct even for HDR consumers. The spec leaves that up to the server, however,
hence it being "loosely defined."

> For a linear color space using FP format, I wonder if you could define a
> function that would result in reliably transferring 256 levels in and
> out of each primary?

I don't see why not, but the same points about HDR consumers apply.

> The principle reason I ask is that if pixel values can be reliably
> stored in deep visuals, then the existing testing infrastructure can be
> used to validate the implementation.

Good point. The only part that really matters there is that there is a fixed 1:1
mapping between the SDR pixel values and a set of HDR pixel values. As to
whether the pixel values correspond to similar shades in both formats shouldn't
matter to core operations, but would to the HDR display.

The point about the 1:1 mapping between pixel values isn't very explicit in the
current wording of the spec, but could be included as an explicit contraint on
the server's transfer function.

I could see that complicating things slightly for implementations that want to
preserve accurate color both when displayed as HDR and when interpreted as SDR,
but it shouldn't be unachievable, especially since the former case would only be
about looking correct, rather than perfect accuracy.

> > scRGB_Linear describes an scRGB color space with linear OETF. scRGB 
> > uses the
> > same primaries and white point as sRGB, and the linear encoding is best 
> > used
> > with an FP16 pixel format.
> 
> I think you should define "OETF" in the spec; most people reading this
> will not know where to start looking for a definition.

Yes, I can do that.

> > COLORSPACE { type:  COLORSPACETYPE
> >  gamma: FIXED }
> 
> Could you just use a FLOAT here? glx uses IEEE floats all over its
> protocol, so it wouldn't be a new type on the wire. I'm a bit sorry to
> not have used floats for Render; that was done when many smaller
> embedded systems still lacked floating point hardware and we were
> concerned about the performance implications of floats in the rendering
> path. For this extension, there's no performance impact here, and using
> a real float would be better. Heck, if you like, use a 64-bit IEEE float.

Yeah, I chose FIXED because I was using Render as a reference and wasn't aware
of the possibility of using floats in protocol. I agree that that is better.

> > DPCSelectInput
> >
> > window: WINDOW
> > enable: SETofDPCSELECTMASK
> >
> > Errors: Window, Value
> 
> Either this request should always deliver an event or never deliver an
> event. Having it 'sometimes' deliver events seems messy. I'd suggest
> just having it always deliver one of each of the events selected; it's
> easy to implement and, as you say, avoids race conditions when the
> developer combines this with the query in the wrong order. That's
> actually a nice cleanup compared with similar functionality in existing
> functions.

Fair enough.

> > A composite manager must use a color space/encoding supported by the
> > display(s) when compositing into the target window.
> 
> What error is returned if this isn't done? And wouldn't a regular
> application, running in non-composited mode, also have the same
> requirement? After all, a compositing manager is just a regular client
> drawing to a regular window.

The wording should probably be changed to "should." As written, there is no
error if a composite manager chooses something not supported by the display,
just no expectation that the results will be displayed accurately. The
capabilities of the display aren't allowed to change, only the preferences (the
expectation being that the server will convert internally anything that doesn't
match perfectly to the mode on a given display), so there's no excuse for a
compositor to choose an option that the display doesn't 

[PATCH libICE 13/13] make sure buffer is zero filled and report if, allocation failed

2017-10-18 Thread walter harms

 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

[PATCH libICE 12/13] add check for malloc

2017-10-18 Thread walter harms


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 (>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 11/13] add check for mall

2017-10-18 Thread walter harms

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 10/13] add check for malloc

2017-10-18 Thread walter harms
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 09/13] add check for malloc

2017-10-18 Thread walter harms


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 08/13] add check for malloc

2017-10-18 Thread walter harms

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 07/13] add check for malloc

2017-10-18 Thread walter harms

 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 06/13] check malloc return

2017-10-18 Thread walter harms


 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 02/13] make IceProtocolShutdown() more readable

2017-10-18 Thread walter harms

 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

2017-10-18 Thread walter harms


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 02/13] make IceProtocolShutdown() more readable

2017-10-18 Thread walter harms

 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 04/13] check the return of malloc()

2017-10-18 Thread walter harms

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 (
 , 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) 

 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 03/13] save one indentlevel in IceProtocolSetup

2017-10-18 Thread walter harms


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 = >process_msg_info[hisOpcode -
-   iceConn->his_min_opcode];
+process_msg_info = >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 00/13] clean up patches for LibICE

2017-10-18 Thread walter harms
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

2017-10-18 Thread walter harms
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, 

Re: [PATCH mga] Fix mga_device_attributes of mgag200eH3.

2017-10-18 Thread Mathieu Larouche

On 16/10/2017 3:07 AM, Michal Srb wrote:

It was missing value for HAL_chipset. All the following values got shifted by 
one and so the whole record was garbage.
---

Alternatively HAL_chipset could be dropped from everywhere because it does not 
seem to be used.

  src/mga_driver.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/mga_driver.c b/src/mga_driver.c
index e496d0b..ab93c71 100644
--- a/src/mga_driver.c
+++ b/src/mga_driver.c
@@ -426,7 +426,7 @@ static const struct mga_device_attributes attribs[] = {
16384, 0x4000,  /* Memory probe size & offset values */
  },
  
-[17] = { 0, 1, 0, 0, 1, 0, 0, new_BARs,

+[17] = { 0, 1, 0, 0, 1, 0, 0, 0, new_BARs,
  (TRANSC_SOLID_FILL | TWO_PASS_COLOR_EXPAND | 
USE_LINEAR_EXPANSION),
{
{ 5, 23 }, /* System VCO frequencies */
This is correct, there's an error there. Thank you for your finding and 
I'm still wondering how it went through. In addition, we could get rid 
of HAL_chipset since there's no plan to bring it back.


Mathieu
___
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