Re: [PATCH] Replace padlength tables with inline functions from misc.h

2012-07-05 Thread Peter Hutterer 
On Mon, Jul 02, 2012 at 08:30:49PM -0700, Alan Coopersmith wrote:
 Adds new function padding_for_int32() and uses existing pad_to_int32()
 depending on required results.
 
 Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com
 ---
 
 On 07/ 2/12 10:58 AM, Keith Packard wrote:
  (also, we should get rid of the padlength array and just use
  '-count  3' instead :-)
 
 Here you go.   I'm not thrilled about the padding_for_int32() name, but
 haven't thought of a better one yet, so feel free to suggest a more
 aesthetically pleasing color for this bike shed.

only comment would be padlength_to_int32, but english prepositions have
never been my forte.

Reviewed-by: Peter Hutterer peter.hutte...@who-t.net, though blurry-eyed
by now

Cheers,
  Peter

  dix/dispatch.c   |4 +---
  include/misc.h   |   14 ++
  os/io.c  |6 ++
  randr/rrscreen.c |5 +
  test/input.c |   34 --
  5 files changed, 50 insertions(+), 13 deletions(-)
 
 diff --git a/dix/dispatch.c b/dix/dispatch.c
 index 73b22dd..ed33a8e 100644
 --- a/dix/dispatch.c
 +++ b/dix/dispatch.c
 @@ -466,8 +466,6 @@ Dispatch(void)
  static int VendorRelease = VENDOR_RELEASE;
  static char *VendorString = VENDOR_NAME;
  
 -static const int padlength[4] = { 0, 3, 2, 1 };
 -
  void
  SetVendorRelease(int release)
  {
 @@ -528,7 +526,7 @@ CreateConnectionBlock(void)
  memmove(pBuf, VendorString, (int) setup.nbytesVendor);
  sizesofar += setup.nbytesVendor;
  pBuf += setup.nbytesVendor;
 -i = padlength[setup.nbytesVendor  3];
 +i = padding_for_int32(setup.nbytesVendor);
  sizesofar += i;
  while (--i = 0)
  *pBuf++ = 0;
 diff --git a/include/misc.h b/include/misc.h
 index fea74b8..c2d146d 100644
 --- a/include/misc.h
 +++ b/include/misc.h
 @@ -228,6 +228,20 @@ pad_to_int32(const int bytes)
  return (((bytes) + 3)  ~3);
  }
  
 +/**
 + * Calculate padding needed to bring the number of bytes to an even
 + * multiple of 4.
 + * @param bytes The minimum number of bytes needed.
 + * @return The bytes of padding needed to arrive at the closest multiple of 4
 + * that is equal or higher than bytes.
 + */
 +static inline int
 +padding_for_int32(const int bytes)
 +{
 +return ((-bytes)  3);
 +}
 +
 +
  extern char **xstrtokenize(const char *str, const char *separators);
  
  /**
 diff --git a/os/io.c b/os/io.c
 index 8d0e5cc..e44db39 100644
 --- a/os/io.c
 +++ b/os/io.c
 @@ -578,8 +578,6 @@ ResetCurrentRequest(ClientPtr client)
  }
  }
  
 -static const int padlength[4] = { 0, 3, 2, 1 };
 -
   /
   * FlushAllOutput()
   *Flush all clients with output.  However, if some client still
 @@ -757,7 +755,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
  oc-output = oco;
  }
  
 -padBytes = padlength[count  3];
 +padBytes = padding_for_int32(count);
  
  if (ReplyCallback) {
  ReplyInfoRec replyinfo;
 @@ -850,7 +848,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void 
 *__extraBuf, int extraCount)
  if (!oco)
  return 0;
  written = 0;
 -padsize = padlength[extraCount  3];
 +padsize = padding_for_int32(extraCount);
  notWritten = oco-count + extraCount + padsize;
  todo = notWritten;
  while (notWritten) {
 diff --git a/randr/rrscreen.c b/randr/rrscreen.c
 index 67cf0ab..fc341b4 100644
 --- a/randr/rrscreen.c
 +++ b/randr/rrscreen.c
 @@ -22,8 +22,6 @@
  
  #include randrstr.h
  
 -static const int padlength[4] = { 0, 3, 2, 1 };
 -
  static CARD16
   RR10CurrentSizeID(ScreenPtr pScreen);
  
 @@ -46,8 +44,7 @@ RREditConnectionInfo(ScreenPtr pScreen)
  connSetup = (xConnSetup *) ConnectionInfo;
  vendor = (char *) connSetup + sizeof(xConnSetup);
  formats = (xPixmapFormat *) ((char *) vendor +
 - connSetup-nbytesVendor +
 - padlength[connSetup-nbytesVendor  3]);
 + pad_to_int32(connSetup-nbytesVendor));
  root = (xWindowRoot *) ((char *) formats +
  sizeof(xPixmapFormat) *
  screenInfo.numPixmapFormats);
 diff --git a/test/input.c b/test/input.c
 index 90ab9ae..191c817 100644
 --- a/test/input.c
 +++ b/test/input.c
 @@ -965,6 +965,19 @@ test_pad_to_int32(int i)
  }
  
  static void
 +test_padding_for_int32(int i)
 +{
 +static const int padlength[4] = { 0, 3, 2, 1 };
 +int expected_bytes = (((i + 3) / 4) * 4) - i;
 +
 +assert(padding_for_int32(i) = 0);
 +assert(padding_for_int32(i) = 3);
 +assert(padding_for_int32(i) == expected_bytes);
 +assert(padding_for_int32(i) == padlength[i  3]);
 +assert((padding_for_int32(i) + i) == pad_to_int32(i));
 +}
 +
 +static void
  include_byte_padding_macros(void)
  {
  printf(Testing bits_to_bytes()\n);
 @@ -996,12 +1009,12 @@ include_byte_padding_macros(void)
  test_bytes_to_int32(INT_MAX - 4);

[PATCH] Replace padlength tables with inline functions from misc.h

2012-07-02 Thread Alan Coopersmith 
Adds new function padding_for_int32() and uses existing pad_to_int32()
depending on required results.

Signed-off-by: Alan Coopersmith alan.coopersm...@oracle.com
---

On 07/ 2/12 10:58 AM, Keith Packard wrote:
 (also, we should get rid of the padlength array and just use
 '-count  3' instead :-)

Here you go.   I'm not thrilled about the padding_for_int32() name, but
haven't thought of a better one yet, so feel free to suggest a more
aesthetically pleasing color for this bike shed.

 dix/dispatch.c   |4 +---
 include/misc.h   |   14 ++
 os/io.c  |6 ++
 randr/rrscreen.c |5 +
 test/input.c |   34 --
 5 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/dix/dispatch.c b/dix/dispatch.c
index 73b22dd..ed33a8e 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -466,8 +466,6 @@ Dispatch(void)
 static int VendorRelease = VENDOR_RELEASE;
 static char *VendorString = VENDOR_NAME;
 
-static const int padlength[4] = { 0, 3, 2, 1 };
-
 void
 SetVendorRelease(int release)
 {
@@ -528,7 +526,7 @@ CreateConnectionBlock(void)
 memmove(pBuf, VendorString, (int) setup.nbytesVendor);
 sizesofar += setup.nbytesVendor;
 pBuf += setup.nbytesVendor;
-i = padlength[setup.nbytesVendor  3];
+i = padding_for_int32(setup.nbytesVendor);
 sizesofar += i;
 while (--i = 0)
 *pBuf++ = 0;
diff --git a/include/misc.h b/include/misc.h
index fea74b8..c2d146d 100644
--- a/include/misc.h
+++ b/include/misc.h
@@ -228,6 +228,20 @@ pad_to_int32(const int bytes)
 return (((bytes) + 3)  ~3);
 }
 
+/**
+ * Calculate padding needed to bring the number of bytes to an even
+ * multiple of 4.
+ * @param bytes The minimum number of bytes needed.
+ * @return The bytes of padding needed to arrive at the closest multiple of 4
+ * that is equal or higher than bytes.
+ */
+static inline int
+padding_for_int32(const int bytes)
+{
+return ((-bytes)  3);
+}
+
+
 extern char **xstrtokenize(const char *str, const char *separators);
 
 /**
diff --git a/os/io.c b/os/io.c
index 8d0e5cc..e44db39 100644
--- a/os/io.c
+++ b/os/io.c
@@ -578,8 +578,6 @@ ResetCurrentRequest(ClientPtr client)
 }
 }
 
-static const int padlength[4] = { 0, 3, 2, 1 };
-
  /
  * FlushAllOutput()
  *Flush all clients with output.  However, if some client still
@@ -757,7 +755,7 @@ WriteToClient(ClientPtr who, int count, const void *__buf)
 oc-output = oco;
 }
 
-padBytes = padlength[count  3];
+padBytes = padding_for_int32(count);
 
 if (ReplyCallback) {
 ReplyInfoRec replyinfo;
@@ -850,7 +848,7 @@ FlushClient(ClientPtr who, OsCommPtr oc, const void 
*__extraBuf, int extraCount)
 if (!oco)
 return 0;
 written = 0;
-padsize = padlength[extraCount  3];
+padsize = padding_for_int32(extraCount);
 notWritten = oco-count + extraCount + padsize;
 todo = notWritten;
 while (notWritten) {
diff --git a/randr/rrscreen.c b/randr/rrscreen.c
index 67cf0ab..fc341b4 100644
--- a/randr/rrscreen.c
+++ b/randr/rrscreen.c
@@ -22,8 +22,6 @@
 
 #include randrstr.h
 
-static const int padlength[4] = { 0, 3, 2, 1 };
-
 static CARD16
  RR10CurrentSizeID(ScreenPtr pScreen);
 
@@ -46,8 +44,7 @@ RREditConnectionInfo(ScreenPtr pScreen)
 connSetup = (xConnSetup *) ConnectionInfo;
 vendor = (char *) connSetup + sizeof(xConnSetup);
 formats = (xPixmapFormat *) ((char *) vendor +
- connSetup-nbytesVendor +
- padlength[connSetup-nbytesVendor  3]);
+ pad_to_int32(connSetup-nbytesVendor));
 root = (xWindowRoot *) ((char *) formats +
 sizeof(xPixmapFormat) *
 screenInfo.numPixmapFormats);
diff --git a/test/input.c b/test/input.c
index 90ab9ae..191c817 100644
--- a/test/input.c
+++ b/test/input.c
@@ -965,6 +965,19 @@ test_pad_to_int32(int i)
 }
 
 static void
+test_padding_for_int32(int i)
+{
+static const int padlength[4] = { 0, 3, 2, 1 };
+int expected_bytes = (((i + 3) / 4) * 4) - i;
+
+assert(padding_for_int32(i) = 0);
+assert(padding_for_int32(i) = 3);
+assert(padding_for_int32(i) == expected_bytes);
+assert(padding_for_int32(i) == padlength[i  3]);
+assert((padding_for_int32(i) + i) == pad_to_int32(i));
+}
+
+static void
 include_byte_padding_macros(void)
 {
 printf(Testing bits_to_bytes()\n);
@@ -996,12 +1009,12 @@ include_byte_padding_macros(void)
 test_bytes_to_int32(INT_MAX - 4);
 test_bytes_to_int32(INT_MAX - 3);
 
-printf(Testing pad_to_int32\n);
+printf(Testing pad_to_int32()\n);
 
 test_pad_to_int32(0);
-test_pad_to_int32(0);
 test_pad_to_int32(1);
 test_pad_to_int32(2);
+test_pad_to_int32(3);
 test_pad_to_int32(7);
 test_pad_to_int32(8);
 test_pad_to_int32(0xFF);
@@ -1012,6 +1025,23 @@ include_byte_padding_macros(void)

Re: [PATCH] Replace padlength tables with inline functions from misc.h

2012-07-02 Thread Keith Packard 
Alan Coopersmith alan.coopersm...@oracle.com writes:

 Adds new function padding_for_int32() and uses existing pad_to_int32()
 depending on required results.

Whoa. Even tests! Totally awesome!

Reviewed-by: Keith Packard kei...@keithp.com

-- 
keith.pack...@intel.com


pgp7WPwqVTc9i.pgp
Description: PGP signature
___
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