Re: [RFC] DeepColor Visual Class Extension
On Sun, 15 Oct 2017, Keith Packard wrote: > Alex Goinswrites: > > > 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
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
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
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
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
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
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
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
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
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 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 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 ( , 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
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
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,
Re: [PATCH mga] Fix mga_device_attributes of mgag200eH3.
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