Skip to content

Commit

Permalink
open-osd: osd-target: Reserved fields should be zero
Browse files Browse the repository at this point in the history
Last patch to attribute-lists added a reserved field at list-item-header.
These should be zeroed out.

While at it:
* Use "struct list_entry" already defined instead of banging directly
  onto buffer with field offsets.
* change all {set,get}_hton*_{le,be} to receive "void *" instead of
  "uint8_t *" so addresses of typed members can be used without a cast.
* Some one used {set,get}_hton*_le version directly in code, instead of
  the {set,get}_hton* macros, so it means it will not work on BE machines.
  (It look like the programmer confused the le_XXX use at some target
   functions that means List-Entry, but above _le means Little-Endian,
   and the two concepts were confused)
* Remove castes to "uint8_t *" in calls to above functions that are no
  longer needed

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
  • Loading branch information
Boaz Harrosh committed Apr 27, 2009
1 parent cf1c989 commit 531b08c
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 120 deletions.
14 changes: 8 additions & 6 deletions osd-target/list-entry.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ int le_pack_attr(void *buf, uint32_t buflen, uint32_t page, uint32_t number,
uint16_t valen, const void *val)
{
uint8_t pad = 0;
struct list_entry *list_entry = buf;
uint8_t *cp = buf;
uint32_t len = buflen;

Expand All @@ -50,11 +51,12 @@ int le_pack_attr(void *buf, uint32_t buflen, uint32_t page, uint32_t number,
if (buflen < LE_MIN_ITEM_LEN)
return -EOVERFLOW;

set_htonl_le(&cp[LE_PAGE_OFF], page);
set_htonl_le(&cp[LE_NUMBER_OFF], number);
set_htonl(&list_entry->page, page);
set_htonl(&list_entry->number, number);

/* length field is not modified to reflect truncation, sec 5.2.2.2 */
set_htons_le(&cp[LE_LEN_OFF], valen);
set_htons(&list_entry->len, valen);
memset(list_entry->reserved, 0, sizeof(list_entry->reserved));

if (val != NULL) {
buflen -= LE_VAL_OFF;
Expand All @@ -70,8 +72,8 @@ int le_pack_attr(void *buf, uint32_t buflen, uint32_t page, uint32_t number,
pad = roundup8(len) - len;
cp += len;
len += pad;
while (pad--)
*cp = 0, cp++;
if (pad)
memset(cp, 0, pad);
return len;
}

Expand All @@ -93,7 +95,7 @@ int le_multiobj_pack_attr(void *buf, uint32_t buflen, uint64_t oid,
if (buflen < MLE_MIN_ITEM_LEN)
return -EOVERFLOW;

set_htonll_le(cp, oid);
set_htonll(cp, oid);

/*
* test if layout of struct multiobj_list_entry is similar to
Expand Down
14 changes: 7 additions & 7 deletions osd-target/osd.c
Original file line number Diff line number Diff line change
Expand Up @@ -295,13 +295,13 @@ static int get_ccap(struct osd_device *osd, void *outbuf, uint64_t outlen,
}

memset(cp, 0, CCAP_TOTAL_LEN);
set_htonl_le(&cp[0], CUR_CMD_ATTR_PG);
set_htonl_le(&cp[4], CCAP_TOTAL_LEN - 8);
set_htonl(&cp[0], CUR_CMD_ATTR_PG);
set_htonl(&cp[4], CCAP_TOTAL_LEN - 8);
memcpy(&cp[CCAP_RICV_OFF], osd->ccap.ricv, sizeof(osd->ccap.ricv));
cp[CCAP_OBJT_OFF] = osd->ccap.obj_type;
set_htonll_le(&cp[CCAP_PID_OFF], osd->ccap.pid);
set_htonll_le(&cp[CCAP_OID_OFF], osd->ccap.oid);
set_htonll_le(&cp[CCAP_APPADDR_OFF], osd->ccap.append_off);
set_htonll(&cp[CCAP_PID_OFF], osd->ccap.pid);
set_htonll(&cp[CCAP_OID_OFF], osd->ccap.oid);
set_htonll(&cp[CCAP_APPADDR_OFF], osd->ccap.append_off);
*used_outlen = CCAP_TOTAL_LEN;

out:
Expand Down Expand Up @@ -418,8 +418,8 @@ static int get_utsap(struct osd_device *osd, uint64_t pid, uint64_t oid,
}

memset(cp, 0, UTSAP_TOTAL_LEN);
set_htonl_le(&cp[0], USER_TMSTMP_PG);
set_htonl_le(&cp[4], UTSAP_TOTAL_LEN - 8);
set_htonl(&cp[0], USER_TMSTMP_PG);
set_htonl(&cp[4], UTSAP_TOTAL_LEN - 8);

get_dfile_name(path, osd->root, pid, oid);
memset(&dsb, 0, sizeof(dsb));
Expand Down
54 changes: 27 additions & 27 deletions osd-target/tests/cdb-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ static void set_one_attr_int(struct osd_device *osd, uint64_t pid, uint64_t oid,
.val = &attrval,
};

set_htonll((uint8_t *) &attrval, val);
set_htonll(&attrval, val);
ret = osd_command_set_set_attributes(&cmd, pid, oid);
assert(ret == 0);
ret = osd_command_attr_build(&cmd, &attr, 1);
Expand Down Expand Up @@ -439,7 +439,7 @@ static void set_attr_int(struct osd_device *osd, uint64_t pid, uint64_t oid,
.val = &attrval,
};

set_htonll((uint8_t *) &attrval, val);
set_htonll(&attrval, val);
ret = osd_command_set_set_attributes(&cmd, pid, oid);
assert(ret == 0);
ret = osd_command_attr_build(&cmd, &attr, 1);
Expand Down Expand Up @@ -557,7 +557,7 @@ void test_query(struct osd_device *osd)
.len = 8,
.val = &attrval,
};
set_htonll((uint8_t *) &attrval, cid);
set_htonll(&attrval, cid);
ret = osd_command_set_create(&cmd, pid, oid + i, 1);
assert(ret == 0);
if (!(i == 2 || i == 8)) {
Expand Down Expand Up @@ -647,8 +647,8 @@ void test_query(struct osd_device *osd)
uint64_t min, max;
qll = 0;
min = 40, max= 80;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
memset(buf, 0, 1024);
cp = buf;
cp[0] = 0x0;
Expand Down Expand Up @@ -676,8 +676,8 @@ void test_query(struct osd_device *osd)

/* first query */
min = 100, max = 180;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
memset(buf, 0, 1024);
cp = buf;
cp[0] = 0x0; /* UNION */
Expand All @@ -687,8 +687,8 @@ void test_query(struct osd_device *osd)

/* second query */
min = 200, max = 323;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
set_qce(cp, page, 1, sizeof(min), &min, sizeof(max), &max);
qll += (4+4+4+2+sizeof(min)+2+sizeof(max));
cp += (4+4+4+2+sizeof(min)+2+sizeof(max));
Expand All @@ -712,8 +712,8 @@ void test_query(struct osd_device *osd)

/* first query */
min = 4, max = 100;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
memset(buf, 0, 1024);
cp = buf;
cp[0] = 0x1; /* INTERSECTION */
Expand All @@ -723,8 +723,8 @@ void test_query(struct osd_device *osd)

/* second query */
min = 10, max = 400;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
set_qce(cp, page, 2, sizeof(min), &min, sizeof(max), &max);
qll += (4+4+4+2+sizeof(min)+2+sizeof(max));
cp += (4+4+4+2+sizeof(min)+2+sizeof(max));
Expand All @@ -748,8 +748,8 @@ void test_query(struct osd_device *osd)

/* first query */
min = 130, max = 130;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
memset(buf, 0, 1024);
cp = buf;
cp[0] = 0x0; /* UNION */
Expand All @@ -759,14 +759,14 @@ void test_query(struct osd_device *osd)

/* second query */
min = 150;
set_htonll((uint8_t *)&min, min);
set_htonll(&min, min);
set_qce(cp, page, 1, sizeof(min), &min, 0, NULL);
qll += (4+4+4+2+sizeof(min)+2+0);
cp += (4+4+4+2+sizeof(min)+2+0);

/* third query */
max = 10;
set_htonll((uint8_t *)&max, max);
set_htonll(&max, max);
set_qce(cp, page, 1, 0, NULL, sizeof(max), &max);
qll += (4+4+4+2+0+2+sizeof(max));
cp += (4+4+4+2+0+2+sizeof(max));
Expand Down Expand Up @@ -808,8 +808,8 @@ void test_query(struct osd_device *osd)

/* first query */
min = 41, max = 169;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
memset(buf, 0, 1024);
cp = buf;
cp[0] = 0x0; /* UNION */
Expand Down Expand Up @@ -852,7 +852,7 @@ void test_query(struct osd_device *osd)

/* second query */
min = 140;
set_htonll((uint8_t *)&min, min);
set_htonll(&min, min);
set_qce(cp, page, 1, sizeof(min), &min, 0, NULL);
qll += (4+4+4+2+sizeof(min)+2+0);
cp += (4+4+4+2+sizeof(min)+2+0);
Expand Down Expand Up @@ -888,8 +888,8 @@ void test_query(struct osd_device *osd)
/* second query */
min = 50;
max = 80;
set_htonll((uint8_t *)&min, min);
set_htonll((uint8_t *)&max, max);
set_htonll(&min, min);
set_htonll(&max, max);
set_qce(cp, page, 1, sizeof(min), &min, sizeof(max), &max);
qll += (4+4+4+2+sizeof(min)+2+sizeof(max));
cp += (4+4+4+2+sizeof(min)+2+sizeof(max));
Expand Down Expand Up @@ -1293,7 +1293,7 @@ static void test_attr_vals(uint8_t *cp, struct attribute_list *attrs,

assert(len == attrs[i].len);
if (len == 8) {
assert(get_ntohll((uint8_t *)attrs[i].val) ==
assert(get_ntohll(attrs[i].val) ==
get_ntohll(cp));
} else if (len != 0) {
assert(memcmp(attrs[i].val, cp, len) == 0);
Expand Down Expand Up @@ -1363,7 +1363,7 @@ void test_set_member_attributes(struct osd_device *osd)
.len = sizeof(attrval),
.val = &attrval,
};
set_htonll((uint8_t *) &attrval, cid);
set_htonll(&attrval, cid);
ret = osd_command_set_set_attributes(&cmd, pid, oid + i);
assert(ret == 0);
ret = osd_command_attr_build(&cmd, &attr, 1);
Expand All @@ -1378,9 +1378,9 @@ void test_set_member_attributes(struct osd_device *osd)
/* set attr on collection members */
uint64_t val1 = 123454321, val2 = 987654, val3 = 59999999;
char str1[MAXNAMELEN], str2[MAXNAMELEN], str3[MAXNAMELEN];
set_htonll((uint8_t *) &val1, val1);
set_htonll((uint8_t *) &val2, val2);
set_htonll((uint8_t *) &val3, val3);
set_htonll(&val1, val1);
set_htonll(&val2, val2);
set_htonll(&val3, val3);
sprintf(str1, "GoMtI");
sprintf(str2, "DeViL");
sprintf(str3, "homeopath");
Expand Down
20 changes: 10 additions & 10 deletions osd-target/tests/db-test.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,9 @@ static void test_attr(struct osd_device *osd)
#ifndef NDEBUG
/* ifdef to avoid unused warning */
struct list_entry *ent = (struct list_entry *)val;
assert(get_ntohl((uint8_t *)&ent->page) == 2);
assert(get_ntohl((uint8_t *)&ent->number) == 12);
assert(get_ntohs((uint8_t *)&ent->len) == strlen(attr)+1);
assert(get_ntohl(&ent->page) == 2);
assert(get_ntohl(&ent->number) == 12);
assert(get_ntohs(&ent->len) == strlen(attr)+1);
assert(strcmp((char *)ent + LE_VAL_OFF, attr) == 0);
#endif

Expand Down Expand Up @@ -307,16 +307,16 @@ static void test_coll(struct osd_device *osd)
assert(usedlen == 4*8);
assert(addlen == 4*8);
assert(cont_id == 0);
assert(get_ntohll((uint8_t *)&oids[0]) == 0x2222);
assert(get_ntohll((uint8_t *)&oids[1]) == 0x3333333333333333);
assert(get_ntohll((uint8_t *)&oids[2]) == 0x7888888888888888);
assert(get_ntohll((uint8_t *)&oids[3]) == 0x7AAAAAAAAAAAAAAA);
assert(get_ntohll(&oids[0]) == 0x2222);
assert(get_ntohll(&oids[1]) == 0x3333333333333333);
assert(get_ntohll(&oids[2]) == 0x7888888888888888);
assert(get_ntohll(&oids[3]) == 0x7AAAAAAAAAAAAAAA);

/*
* XXX: following is sqlite bug. sqlite converts uint64_t to double
* and looses precision
*/
assert(get_ntohll((uint8_t *)&oids[4]) != 0xFFFFFFFFFFFFFFFF);
assert(get_ntohll(&oids[4]) != 0xFFFFFFFFFFFFFFFF);

/* test empty cid */
ret = coll_isempty_cid(osd->dbc, 0x20, 0x1, &isempty);
Expand Down Expand Up @@ -377,8 +377,8 @@ static void test_copy_coll(struct osd_device *osd)
assert(usedlen == 2*8);
assert(addlen == 2*8);
assert(cont_id == 0);
assert(get_ntohll((uint8_t *)&oids[0]) == 0x111);
assert(get_ntohll((uint8_t *)&oids[1]) == 0x1111111111111111);
assert(get_ntohll(&oids[0]) == 0x111);
assert(get_ntohll(&oids[1]) == 0x1111111111111111);
}

int main()
Expand Down
Loading

0 comments on commit 531b08c

Please sign in to comment.