nklein / userial

Library for binary serialization/unserialization
http://nklein.com/software/unet/userial/
MIT License
18 stars 0 forks source link

Standard signed integer de/serializers #4

Closed bahollis closed 13 years ago

bahollis commented 13 years ago

This project is looking very nice, but I have 1 small criticism. That being the rather non-standard standard signed integer serializers. I cannot come up with a good reason for why you chose the scheme of 2's compliment AND an explicit sign bit.

This rules out communicating with any other client/server that does not implement this rare scheme. Forcing the end user to define a proper serializer for straight up 2's compliment integers. I think it would be a better idea to implement the standard signed integer serializer as just 2's compliment, as this is the representation used by the vast majority of network protocols.

I have modified the make-int-serializer macro to implement standard 2's compliment. I am sure there is a more optimal way of doing it, but this is what I could do in the 10 minutes I was willing to devote to it ^^

(defmacro make-int-serializer (key bytes)
  "Make SERIALIZE/UNSERIALIZE methods for a 2's compliment signed-int of BYTES bytes in length dispatched by KEY."
  `(progn
     (defmethod serialize ((type (eql ,key)) value &key (buffer *buffer*))
       (declare (type buffer buffer)
                (ignore type)
                (type (signed-byte ,(* bytes 8)) value))
       (unroll-add-bytes buffer value ,bytes))
     (defmethod unserialize ((type (eql ,key)) &key (buffer *buffer*))
       (declare (type buffer buffer)
                (ignore type))
       (let ((value (unroll-get-bytes buffer ,bytes)))
          (declare (type (unsigned-byte ,(* bytes 8)) value))    
          (values (if (logbitp ,(1- (* 8 bytes)) value)
                      (let ((negval -1))
                        (setf (ldb (byte ,(* 8 bytes) 0) negval) value)
                        negval)
                      value)
                  buffer)))))
nklein commented 13 years ago

You are correct. I could easily have done two's complement. I will correct that in the next release.... which will hopefully be later this week.

nklein commented 13 years ago

As for why I chose the scheme that I did... I was thinking.... I know exactly what (ldb ...) will return on the high bytes of (+ x offset) and I don't know what it will return on the high bytes of (- x) for things larger than a fixnum.

That said... I should have realized that I knew exactly the number of bits involved and could easily have done the "flip-the-bits-and-add-one" thing.

bahollis commented 13 years ago

I look forward to the next release ^^

nklein commented 13 years ago

Completed the fix in github. Tarball release coming very soon.