Remove unnecessary Item() constuctor couchbase#2

Remove another constructor of Item, the one which takes only a data
size - i.e. allocates a value but doesn't copy it in. In all existing
use-cases this can be replaced with either:

a) The constructor which does copy the data in, as the use-case was
   doing performing the copy in two stages (construct Item of correct
   size, then memcpy()).

b) The consturctor which takes both ptr and size, with with ptr==NULL
   - this is already handled by the setData() private method of Item.

Change-Id: Iffbfef423e82bb021f74860a72dc7b5936bf2215
Reviewed-on: http://review.couchbase.org/42625
Reviewed-by: Michael Wiederhold <[email protected]>
Reviewed-by: Trond Norbye <[email protected]>
Reviewed-by: abhinav dangeti <[email protected]>
Tested-by: abhinav dangeti <[email protected]>

Author: daverigby

src/couch-kvstore/couch-kvstore.cc

@@ -1506,7 +1506,7 @@ couchstore_error_t CouchKVStore::fetchDoc(Db *db, DocInfo *docinfo,
 
     if (metaOnly || (fetchDelete && docinfo->deleted)) {
         Item *it = new Item(docinfo->id.buf, (size_t)docinfo->id.size,
-                            docinfo->size, itemFlags, (time_t)exptime,
+                            itemFlags, (time_t)exptime, NULL, docinfo->size,
                             ext_meta, ext_len, cas, docinfo->db_seq, vbId);
         if (docinfo->deleted) {
             it->setDeleted();

src/ep_engine.cc

@@ -5020,8 +5020,8 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::setWithMeta(const void* cookie,
     uint8_t ext_meta[1];
     uint8_t ext_len = EXT_META_LEN;
     *(ext_meta) = datatype;
-    Item *itm = new Item(key, keylen, vallen, flags, expiration, ext_meta, ext_len,
-                         cas, -1, vbucket);
+    Item *itm = new Item(key, keylen, flags, expiration, dta, vallen,
+                         ext_meta, ext_len, cas, -1, vbucket);
 
     if (itm == NULL) {
         return sendResponse(response, NULL, 0, NULL, 0, NULL, 0,
@@ -5038,7 +5038,6 @@ ENGINE_ERROR_CODE EventuallyPersistentEngine::setWithMeta(const void* cookie,
     }
 
     itm->setRevSeqno(seqno);
-    memcpy((char*)itm->getData(), dta, vallen);
 
     bool allowExisting = (opcode == PROTOCOL_BINARY_CMD_SET_WITH_META ||
                           opcode == PROTOCOL_BINARY_CMD_SETQ_WITH_META);
@@ -5407,7 +5406,7 @@ EventuallyPersistentEngine::returnMeta(const void* cookie,
         uint8_t ext_meta[1];
         uint8_t ext_len = EXT_META_LEN;
         *(ext_meta) = datatype;
-        Item *itm = new Item(key, keylen, vallen, flags, exp, ext_meta,
+        Item *itm = new Item(key, keylen, flags, exp, dta, vallen, ext_meta,
                              ext_len, cas, -1, vbucket);
 
         if (!itm) {
@@ -5416,7 +5415,6 @@ EventuallyPersistentEngine::returnMeta(const void* cookie,
                                 PROTOCOL_BINARY_RESPONSE_ENOMEM, 0, cookie);
         }
 
-        memcpy((char*)itm->getData(), dta, vallen);
         if (mutate_type == SET_RET_META) {
             ret = epstore->set(*itm, cookie);
         } else {

src/ep_engine.h

@@ -199,7 +199,7 @@ class EventuallyPersistentEngine : public ENGINE_HANDLE_V1 {
         uint8_t ext_meta[1];
         uint8_t ext_len = EXT_META_LEN;
         *(ext_meta) = datatype;
-        *itm = new Item(key, nkey, nbytes, flags, expiretime, ext_meta,
+        *itm = new Item(key, nkey, flags, expiretime, NULL, nbytes, ext_meta,
                         ext_len);
         if (*itm == NULL) {
             return memoryCondition();

src/item.h

@@ -285,23 +285,6 @@ class ItemMetaData {
 class Item : public RCValue {
 public:
 
-    Item(const void* k, const size_t nk, const size_t nb,
-         const uint32_t fl, const time_t exp, uint8_t* ext_meta = NULL,
-         uint8_t ext_len = 0, uint64_t theCas = 0, int64_t i = -1,
-         uint16_t vbid = 0, uint8_t nru_value = INITIAL_NRU_VALUE) :
-        metaData(theCas, 1, fl, exp),
-        key(static_cast<const char*>(k), nk),
-        bySeqno(i),
-        queuedTime(ep_current_time()),
-        vbucketId(vbid),
-        op(queue_op_set),
-        nru(nru_value)
-    {
-        cb_assert(bySeqno != 0);
-        setData(NULL, nb, ext_meta, ext_len);
-        ObjectRegistry::onCreateItem(this);
-    }
-
     Item(const std::string &k, const uint32_t fl, const time_t exp,
          const value_t &val, uint64_t theCas = 0,  int64_t i = -1,
          uint16_t vbid = 0, uint64_t sno = 1, uint8_t nru_value = INITIAL_NRU_VALUE) :

src/stored-value.cc

@@ -582,8 +582,8 @@ add_type_t HashTable::unlocked_addTempItem(int &bucket_num,
     uint8_t ext_meta[1];
     uint8_t ext_len = EXT_META_LEN;
     *(ext_meta) = PROTOCOL_BINARY_RAW_BYTES;
-    Item itm(key.c_str(), key.length(), (size_t)0, (uint32_t)0, (time_t)0,
-             ext_meta, ext_len, 0, StoredValue::state_temp_init);
+    Item itm(key.c_str(), key.length(), /*flags*/0, /*exp*/0, /*data*/NULL,
+             /*size*/0, ext_meta, ext_len, 0, StoredValue::state_temp_init);
 
     // if a temp item for a possibly deleted, set it non-resident by resetting
     // the value cuz normally a new item added is considered resident which

src/tapconnection.cc

@@ -1870,8 +1870,11 @@ Item* TapProducer::getNextItem(const void *c, uint16_t *vbucket, uint16_t &ret,
                 ret = TAP_MUTATION;
             } else if (r == ENGINE_KEY_ENOENT) {
                 // Item was deleted and set a message type to tap_deletion.
-                itm = new Item(qi->getKey().c_str(), qi->getNKey(), 0,
-                               0, 0, NULL, 0, 0, -1, qi->getVBucketId());
+                itm = new Item(qi->getKey().c_str(), qi->getNKey(),
+                               /*flags*/0, /*exp*/0,
+                               /*data*/NULL, /*size*/0,
+                               /*ext_meta*/NULL, /*ext_len*/0,
+                               /*cas*/0, /*seqno*/-1, qi->getVBucketId());
                 itm->setRevSeqno(qi->getRevSeqno());
                 ret = TAP_DELETION;
             } else if (r == ENGINE_EWOULDBLOCK) {
@@ -1900,8 +1903,11 @@ Item* TapProducer::getNextItem(const void *c, uint16_t *vbucket, uint16_t &ret,
             }
             ++stats.numTapFGFetched;
         } else if (qi->getOperation() == queue_op_del) {
-            itm = new Item(qi->getKey().c_str(), qi->getNKey(), 0,
-                           0, 0, NULL, 0, qi->getCas(), -1, qi->getVBucketId());
+            itm = new Item(qi->getKey().c_str(), qi->getNKey(),
+                           /*flags*/0, /*exp*/0,
+                           /*data*/NULL, /*size*/0,
+                           /*ext_meta*/NULL, /*ext_len*/0,
+                           qi->getCas(), /*seqno*/-1, qi->getVBucketId());
             itm->setRevSeqno(qi->getRevSeqno());
             ret = TAP_DELETION;
             ++stats.numTapDeletes;