I think so, snap_id is defined and used as uint64_t in ceph, but here
static_cast may introduce bug, since two snap_id may get the same
prefix, then same key in snap_mapper.
diff --git a/src/osd/SnapMapper.cc b/src/osd/SnapMapper.cc
index 315e2e2..27cc2b7 100644
--- a/src/osd/SnapMapper.cc
+++ b/src/osd/SnapMapper.cc
@@ -72,8 +72,7 @@ string SnapMapper::get_prefix(snapid_
t snap)
char buf[100];
int len = snprintf(
buf, sizeof(buf),
- "%.*X_", (int)(sizeof(snap)*2),
- static_cast<unsigned>(snap));
+ "%.*llX_", (int)(sizeof(snap)*2), snap);
2015-02-04 1:25 GMT+08:00 Samuel Just <sam.just@xxxxxxxxxxx>:
> Should probably be cast to long unsigned with lX conversion specifier?
> -Sam
>
> On Tue, Feb 3, 2015 at 9:21 AM, Samuel Just <sam.just@xxxxxxxxxxx> wrote:
>> It looks like snapid_t is a uint64_t, but snprintf expects an unsigned there.
>> -Sam
>>
>> On Tue, Feb 3, 2015 at 9:15 AM, Gregory Farnum <greg@xxxxxxxxxxx> wrote:
>>> On Tue, Feb 3, 2015 at 4:12 AM, Ding Dinghua <dingdinghua85@xxxxxxxxx> wrote:
>>>> Hi all:
>>>> I don't understand why SnapMapper::get_prefix static_cast snap
>>>> to unsigned:
>>>>
>>>> string SnapMapper::get_prefix(snapid_t snap)
>>>> {
>>>> char buf[100];
>>>> int len = snprintf(
>>>> buf, sizeof(buf),
>>>> "%.*X_", (int)(sizeof(snap)*2),
>>>> static_cast<unsigned>(snap));
>>>> return MAPPING_PREFIX + string(buf, len);
>>>> }
>>>>
>>>> Will this limit snapshot count in pool to 2^32 -1 ?
>>>>
>>>> Could anyone clarify ? Thanks
>>>
>>> I think the code base is a little confused about whether snaps should
>>> be 32 or 64 bits in various places. :( That said, the size of unsigned
>>> can vary across architectures, so this should probably be sized more
>>> explicitly as whatever it's supposed to be on the disk...
--
Ding Dinghua
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
