> Author: stefan2
> Date: Wed Jul 24 13:24:44 2013
> New Revision: 1506545
>
> URL: http://svn.apache.org/r1506545
> Log:
> On the fsfs-improvements branch: Finally switch to a new FSFS ID API.
>
> This replaces the previous string-based API with one that represents
> IDs as quadruples of <revision,sub-id> pairs of integers. Thus, it
> now matches the implicit usage of the node_id, branch_id, txn_id and
> rev_offset parts of those IDs.
>
> The patch does four things. First, it replaces the current API in
> id.*. Second, it must use the new svn_fs_fs__id_part_t * instead of
> const char * in all FSFS code. In some places we have to add a level
> of indirection as key parts can now be put on stack instead of being
> pool-allocated but we always pass them along through pointers.
>
> Third, structs using a txn ID will use the new data type as well -
> requiring more code to be updated. Lastly, we have to update code
> that (de-)serializes IDs or handles ID assignment and increments.
[...]
> svn_fs_fs__id_eq(const svn_fs_id_t *a,
> const svn_fs_id_t *b)
> {
> - id_private_t *pvta = a->fsap_data, *pvtb = b->fsap_data;
> + fs_fs__id_t *id_a = (fs_fs__id_t *)a;
> + fs_fs__id_t *id_b = (fs_fs__id_t *)b;
>
> if (a == b)
> return TRUE;
> - if (strcmp(pvta->node_id, pvtb->node_id) != 0)
> - return FALSE;
> - if (strcmp(pvta->copy_id, pvtb->copy_id) != 0)
> - return FALSE;
> - if ((pvta->txn_id == NULL) != (pvtb->txn_id == NULL))
> - return FALSE;
> - if (pvta->txn_id && pvtb->txn_id && strcmp(pvta->txn_id, pvtb->txn_id) != 0)
> - return FALSE;
> - if (pvta->rev != pvtb->rev)
> - return FALSE;
> - if (pvta->offset != pvtb->offset)
> - return FALSE;
> - return TRUE;
> +
> + return memcmp(&id_a->private_id, &id_b->private_id,
> + sizeof(id_a->private_id)) == 0;
> }
I am wondering, whether this is a portable way of comparing structs. Isn't
it possible that fs_fs__id_t would compile with padding and hence, result in
false negative checks here?
It looks like we do not always use apr_pcalloc() to zero out the memory for
these structures (e.g. within the subversion/libsvn_fs_fs/temp_deserializer.c
code), so it might be possible that the padding bytes would contain some random
garbage, and we would attempt to compare it with memcmp().
Regards,
Evgeny Kotkov
Received on 2014-08-21 11:45:37 CEST