qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alanconway <...@git.apache.org>
Subject [GitHub] qpid-proton issue #159: PROTON-1940: [c] normalize encoding of multiple="tru...
Date Wed, 26 Sep 2018 16:07:29 GMT
Github user alanconway commented on the issue:

    https://github.com/apache/qpid-proton/pull/159
  
    Changes prompted by astitcher's comments:
    
    - better explanation of mulitple fields, corrected AMQP spec links
    - added block comment for vfill format letters
    - factored out pni_normalize_multiple to reduce visual churn in _vfill() impl
    
    Not changed: still using a new format letter. 2 reasons - consistency and efficiency/ease
of impl:
    
    consistency: the multiple attribute is part of an AMQP composite field definition. The
purpose of _vfill() is to fill in composite fields correctly - so IMO that is where this belongs.
I personally hate the vfill char-based mini-language; it is hard to read, hard to remember
and error-prone. However it seems like something we should replace entirely or live with for
now, rather than have inconsistent approaches for different fields.
    
    ease of impl: as part of vfill, we can copy exactly as much of the source data as we need
to the target. A normalize function outside of vfill would either have to modify the source
value in-place (side-effects on caller's data value) or create an intermediate copy (expensive,
extra memory management) Both are inconsistent with existing use of vfill.
    



---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Mime
View raw message