[Spice-devel,spice-common] Marshaller: rename _add_ref() to _add_by_ref()

Submitted by Jonathon Jongsma on Dec. 5, 2016, 10:19 p.m.

Details

Message ID 1480976344-12523-1-git-send-email-jjongsma@redhat.com
State Accepted
Commit adb36c6185a363c3f5775559fb1dbfba1cdccaf4
Headers show
Series "Marshaller: rename _add_ref() to _add_by_ref()" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Jonathon Jongsma Dec. 5, 2016, 10:19 p.m.
The spice_marshaller_add_ref() family of functions is confusing since it
sounds like you're incrementing a reference on the marshaller. What it
is actually doing is adding a data buffer to the marshaller by reference
rather than by value. Changing the function names to _add_by_ref() makes
this clearer.

The old functions are deprecated and are simply inline functions that
call the new functions.
---
 common/marshaller.c | 14 +++++++-------
 common/marshaller.h | 21 +++++++++++++++++----
 2 files changed, 24 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/common/marshaller.c b/common/marshaller.c
index 00a7123..2fccd52 100644
--- a/common/marshaller.c
+++ b/common/marshaller.c
@@ -317,8 +317,8 @@  void spice_marshaller_unreserve_space(SpiceMarshaller *m, size_t size)
     item->len -= size;
 }
 
-uint8_t *spice_marshaller_add_ref_full(SpiceMarshaller *m, uint8_t *data, size_t size,
-                                       spice_marshaller_item_free_func free_data, void *opaque)
+uint8_t *spice_marshaller_add_by_ref_full(SpiceMarshaller *m, uint8_t *data, size_t size,
+                                          spice_marshaller_item_free_func free_data, void *opaque)
 {
     MarshallerItem *item;
     SpiceMarshallerData *d;
@@ -349,21 +349,21 @@  uint8_t *spice_marshaller_add(SpiceMarshaller *m, const uint8_t *data, size_t si
     return ptr;
 }
 
-uint8_t *spice_marshaller_add_ref(SpiceMarshaller *m, const uint8_t *data, size_t size)
+uint8_t *spice_marshaller_add_by_ref(SpiceMarshaller *m, const uint8_t *data, size_t size)
 {
     /* the cast to no-const here is safe as data is used for writing only if
      * free_data pointer is not NULL
      */
-    return spice_marshaller_add_ref_full(m, (uint8_t *) data, size, NULL, NULL);
+    return spice_marshaller_add_by_ref_full(m, (uint8_t *) data, size, NULL, NULL);
 }
 
-void spice_marshaller_add_ref_chunks(SpiceMarshaller *m, SpiceChunks *chunks)
+void spice_marshaller_add_chunks_by_ref(SpiceMarshaller *m, SpiceChunks *chunks)
 {
     unsigned int i;
 
     for (i = 0; i < chunks->num_chunks; i++) {
-        spice_marshaller_add_ref(m, chunks->chunk[i].data,
-                                 chunks->chunk[i].len);
+        spice_marshaller_add_by_ref(m, chunks->chunk[i].data,
+                                    chunks->chunk[i].len);
     }
 }
 
diff --git a/common/marshaller.h b/common/marshaller.h
index 316184e..9ae1bdf 100644
--- a/common/marshaller.h
+++ b/common/marshaller.h
@@ -38,10 +38,23 @@  void spice_marshaller_destroy(SpiceMarshaller *m);
 uint8_t *spice_marshaller_reserve_space(SpiceMarshaller *m, size_t size);
 void spice_marshaller_unreserve_space(SpiceMarshaller *m, size_t size);
 uint8_t *spice_marshaller_add(SpiceMarshaller *m, const uint8_t *data, size_t size);
-uint8_t *spice_marshaller_add_ref(SpiceMarshaller *m, const uint8_t *data, size_t size);
-uint8_t *spice_marshaller_add_ref_full(SpiceMarshaller *m, uint8_t *data, size_t size,
-                                       spice_marshaller_item_free_func free_data, void *opaque);
-void     spice_marshaller_add_ref_chunks(SpiceMarshaller *m, SpiceChunks *chunks);
+uint8_t *spice_marshaller_add_by_ref(SpiceMarshaller *m, const uint8_t *data, size_t size);
+uint8_t *spice_marshaller_add_by_ref_full(SpiceMarshaller *m, uint8_t *data, size_t size,
+                                          spice_marshaller_item_free_func free_data, void *opaque);
+void spice_marshaller_add_chunks_by_ref(SpiceMarshaller *m, SpiceChunks *chunks);
+SPICE_GNUC_DEPRECATED inline uint8_t *spice_marshaller_add_ref(SpiceMarshaller *m, const uint8_t *data, size_t size)
+{
+    return spice_marshaller_add_by_ref(m, data, size);
+}
+SPICE_GNUC_DEPRECATED inline uint8_t *spice_marshaller_add_ref_full(SpiceMarshaller *m, uint8_t *data, size_t size,
+                                              spice_marshaller_item_free_func free_data, void *opaque)
+{
+    return spice_marshaller_add_by_ref_full(m, data, size, free_data, opaque);
+}
+SPICE_GNUC_DEPRECATED inline void spice_marshaller_add_ref_chunks(SpiceMarshaller *m, SpiceChunks *chunks)
+{
+    spice_marshaller_add_chunks_by_ref(m, chunks);
+}
 void spice_marshaller_flush(SpiceMarshaller *m);
 void spice_marshaller_set_base(SpiceMarshaller *m, size_t base);
 uint8_t *spice_marshaller_linearize(SpiceMarshaller *m, size_t skip,

Comments

On Mon, Dec 05, 2016 at 04:19:04PM -0600, Jonathon Jongsma wrote:
> The spice_marshaller_add_ref() family of functions is confusing since it
> sounds like you're incrementing a reference on the marshaller. What it
> is actually doing is adding a data buffer to the marshaller by reference
> rather than by value. Changing the function names to _add_by_ref() makes
> this clearer.
> 
> The old functions are deprecated and are simply inline functions that
> call the new functions.

Do we have to keep the old ones?

Christophe
On Tue, 2016-12-06 at 10:49 +0100, Christophe Fergeau wrote:
> On Mon, Dec 05, 2016 at 04:19:04PM -0600, Jonathon Jongsma wrote:
> > 
> > The spice_marshaller_add_ref() family of functions is confusing
> > since it
> > sounds like you're incrementing a reference on the marshaller. What
> > it
> > is actually doing is adding a data buffer to the marshaller by
> > reference
> > rather than by value. Changing the function names to _add_by_ref()
> > makes
> > this clearer.
> > 
> > The old functions are deprecated and are simply inline functions
> > that
> > call the new functions.
> 
> Do we have to keep the old ones?
> 
> Christophe


probably not if we update all of the repositories that use the old
function. But I figured it was kinder to do it this way for at least a
little while to ease the transition.

Jonathon
On Tue, Dec 06, 2016 at 09:33:51AM -0600, Jonathon Jongsma wrote:
> On Tue, 2016-12-06 at 10:49 +0100, Christophe Fergeau wrote:
> > On Mon, Dec 05, 2016 at 04:19:04PM -0600, Jonathon Jongsma wrote:
> > > 
> > > The spice_marshaller_add_ref() family of functions is confusing
> > > since it
> > > sounds like you're incrementing a reference on the marshaller. What
> > > it
> > > is actually doing is adding a data buffer to the marshaller by
> > > reference
> > > rather than by value. Changing the function names to _add_by_ref()
> > > makes
> > > this clearer.
> > > 
> > > The old functions are deprecated and are simply inline functions
> > > that
> > > call the new functions.
> > 
> > Do we have to keep the old ones?
> > 
> > Christophe
> 
> 
> probably not if we update all of the repositories that use the old
> function. But I figured it was kinder to do it this way for at least a
> little while to ease the transition.

Ok, fine with me,
Acked-by: Christophe Fergeau <cfergeau@redhat.com>