diff --git a/bucket.go b/bucket.go index 78a68f548..f086a9ddf 100644 --- a/bucket.go +++ b/bucket.go @@ -202,13 +202,18 @@ func (b *Bucket) CreateBucket(key []byte) (rb *Bucket, err error) { // Returns an error if the bucket name is blank, or if the bucket name is too long. // The bucket instance is only valid for the lifetime of the transaction. func (b *Bucket) CreateBucketIfNotExists(key []byte) (rb *Bucket, err error) { + // Insert into node. + // Tip: Use a new variable `newKey` instead of reusing the existing `key` to prevent + // it from being marked as leaking, and accordingly cannot be allocated on stack. + newKey := cloneBytes(key) + lg := b.tx.db.Logger() - lg.Debugf("Creating bucket if not exist %q", string(key)) + lg.Debugf("Creating bucket if not exist %q", string(newKey)) defer func() { if err != nil { - lg.Errorf("Creating bucket if not exist %q failed: %v", string(key), err) + lg.Errorf("Creating bucket if not exist %q failed: %v", string(newKey), err) } else { - lg.Debugf("Creating bucket if not exist %q successfully", string(key)) + lg.Debugf("Creating bucket if not exist %q successfully", string(newKey)) } }() @@ -216,15 +221,10 @@ func (b *Bucket) CreateBucketIfNotExists(key []byte) (rb *Bucket, err error) { return nil, errors.ErrTxClosed } else if !b.tx.writable { return nil, errors.ErrTxNotWritable - } else if len(key) == 0 { + } else if len(newKey) == 0 { return nil, errors.ErrBucketNameRequired } - // Insert into node. - // Tip: Use a new variable `newKey` instead of reusing the existing `key` to prevent - // it from being marked as leaking, and accordingly cannot be allocated on stack. - newKey := cloneBytes(key) - if b.buckets != nil { if child := b.buckets[string(newKey)]; child != nil { return child, nil @@ -269,13 +269,15 @@ func (b *Bucket) CreateBucketIfNotExists(key []byte) (rb *Bucket, err error) { // DeleteBucket deletes a bucket at the given key. // Returns an error if the bucket does not exist, or if the key represents a non-bucket value. func (b *Bucket) DeleteBucket(key []byte) (err error) { + newKey := cloneBytes(key) + lg := b.tx.db.Logger() - lg.Debugf("Deleting bucket %q", string(key)) + lg.Debugf("Deleting bucket %q", string(newKey)) defer func() { if err != nil { - lg.Errorf("Deleting bucket %q failed: %v", string(key), err) + lg.Errorf("Deleting bucket %q failed: %v", string(newKey), err) } else { - lg.Debugf("Deleting bucket %q successfully", string(key)) + lg.Debugf("Deleting bucket %q successfully", string(newKey)) } }() @@ -287,17 +289,17 @@ func (b *Bucket) DeleteBucket(key []byte) (err error) { // Move cursor to correct position. c := b.Cursor() - k, _, flags := c.seek(key) + k, _, flags := c.seek(newKey) // Return an error if bucket doesn't exist or is not a bucket. - if !bytes.Equal(key, k) { + if !bytes.Equal(newKey, k) { return errors.ErrBucketNotFound } else if (flags & common.BucketLeafFlag) == 0 { return errors.ErrIncompatibleValue } // Recursively delete all child buckets. - child := b.Bucket(key) + child := b.Bucket(newKey) err = child.ForEachBucket(func(k []byte) error { if err := child.DeleteBucket(k); err != nil { return fmt.Errorf("delete bucket: %s", err) @@ -309,7 +311,7 @@ func (b *Bucket) DeleteBucket(key []byte) (err error) { } // Remove cached copy. - delete(b.buckets, string(key)) + delete(b.buckets, string(newKey)) // Release all bucket pages to freelist. child.nodes = nil @@ -317,7 +319,7 @@ func (b *Bucket) DeleteBucket(key []byte) (err error) { child.free() // Delete the node if we have a matching key. - c.node().del(key) + c.node().del(newKey) return nil } @@ -328,7 +330,19 @@ func (b *Bucket) DeleteBucket(key []byte) (err error) { // 2. or the key already exists in the destination bucket; // 3. or the key represents a non-bucket value; // 4. the source and destination buckets are the same. -func (b *Bucket) MoveBucket(key []byte, dstBucket *Bucket) error { +func (b *Bucket) MoveBucket(key []byte, dstBucket *Bucket) (err error) { + newKey := cloneBytes(key) + + lg := b.tx.db.Logger() + lg.Debugf("Moving bucket %q", string(newKey)) + defer func() { + if err != nil { + lg.Errorf("Moving bucket %q failed: %v", string(newKey), err) + } else { + lg.Debugf("Moving bucket %q successfully", string(newKey)) + } + }() + if b.tx.db == nil || dstBucket.tx.db == nil { return errors.ErrTxClosed } else if !dstBucket.Writable() { @@ -337,39 +351,41 @@ func (b *Bucket) MoveBucket(key []byte, dstBucket *Bucket) error { // Move cursor to correct position. c := b.Cursor() - k, v, flags := c.seek(key) + k, v, flags := c.seek(newKey) // Return an error if bucket doesn't exist or is not a bucket. - if !bytes.Equal(key, k) { + if !bytes.Equal(newKey, k) { return errors.ErrBucketNotFound } else if (flags & common.BucketLeafFlag) == 0 { - return fmt.Errorf("key %q isn't a bucket in the source bucket: %w", key, errors.ErrIncompatibleValue) + lg.Errorf("An incompatible key %s exists in the source bucket", string(newKey)) + return errors.ErrIncompatibleValue } // Do nothing (return true directly) if the source bucket and the // destination bucket are actually the same bucket. if b == dstBucket || (b.RootPage() == dstBucket.RootPage() && b.RootPage() != 0) { - return fmt.Errorf("source bucket %s and target bucket %s are the same: %w", b.String(), dstBucket.String(), errors.ErrSameBuckets) + lg.Errorf("The source bucket (%s) and the target bucket (%s) are the same bucket", b.String(), dstBucket.String()) + return errors.ErrSameBuckets } // check whether the key already exists in the destination bucket curDst := dstBucket.Cursor() - k, _, flags = curDst.seek(key) + k, _, flags = curDst.seek(newKey) // Return an error if there is an existing key in the destination bucket. - if bytes.Equal(key, k) { + if bytes.Equal(newKey, k) { if (flags & common.BucketLeafFlag) != 0 { return errors.ErrBucketExists } - return fmt.Errorf("key %q already exists in the target bucket: %w", key, errors.ErrIncompatibleValue) + lg.Errorf("An incompatible key %s exists in the target bucket", string(newKey)) + return errors.ErrIncompatibleValue } // remove the sub-bucket from the source bucket - delete(b.buckets, string(key)) - c.node().del(key) + delete(b.buckets, string(newKey)) + c.node().del(newKey) // add te sub-bucket to the destination bucket - newKey := cloneBytes(key) newValue := cloneBytes(v) curDst.node().put(newKey, newKey, newValue, 0, common.BucketLeafFlag)