Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbd: trigger an error on invalid max snaps value #977

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

zxysilent
Copy link
Contributor

@zxysilent zxysilent commented Mar 31, 2024

It is necessary to determine whether cMaxSnaps is a large 0. Otherwise, the following code will panic

// GetSnapshotNames returns more than just the names of snapshots
// associated with the rbd image.
//
// Implements:
//
//	int rbd_snap_list(rbd_image_t image, rbd_snap_info_t *snaps, int *max_snaps);
func (image *Image) GetSnapshotNames() (snaps []SnapInfo, err error) {
	if err := image.validate(imageIsOpen); err != nil {
		return nil, err
	}

	var c_max_snaps C.int

	ret := C.rbd_snap_list(image.image, nil, &c_max_snaps)
 	if c_max_snaps < 1 {
		return nil, RBDError(ret)
	}
	c_snaps := make([]C.rbd_snap_info_t, c_max_snaps)
	snaps = make([]SnapInfo, c_max_snaps)

	ret = C.rbd_snap_list(image.image,
		&c_snaps[0], &c_max_snaps)
	if ret < 0 {
		return nil, RBDError(ret)
	}

	for i, s := range c_snaps {
		snaps[i] = SnapInfo{Id: uint64(s.id),
			Size: uint64(s.size),
			Name: C.GoString(s.name)}
	}

	C.rbd_snap_list_end(&c_snaps[0])
	return snaps[:len(snaps)-1], nil
}

@phlogistonjohn
Copy link
Collaborator

Thanks for the change, on first glance the code seems reasonable. However, the commit message needs updating. Please add your 'Signed-off-by' to the commit and update your commit message to conform to the project's typical flow of <pkg>: <short_desc>. In your case I suggest rbd: trigger an error on invalid max snaps value or something like that. Thanks!

@zxysilent zxysilent changed the title bugfix: fix index out of range(&cSnaps[0]) when use image.GetSnapshotNames rbd: trigger an error on invalid max snaps value Apr 2, 2024
@zxysilent
Copy link
Contributor Author

Thank you, this is my first time submitting an MR for this project

@anoopcs9
Copy link
Collaborator

anoopcs9 commented Apr 2, 2024

Thank you, this is my first time submitting an MR for this project

I did a force push to have a single commit with the change. This way the git history is much cleaner.

@phlogistonjohn
Copy link
Collaborator

@Mergifyio rebase

In file include from rbd.go:931
var cMaxSnaps C.int
ret := C.rbd_snap_list(image.image, nil, &cMaxSnaps)
cSnaps := make([]C.rbd_snap_info_t, cMaxSnaps)
It is necessary to determine whether cMaxSnaps is a large 0. Otherwise, the following code will panic
ret = C.rbd_snap_list(image.image,&cSnaps[0], &cMaxSnaps)

Signed-off-by: zxysilent <[email protected]>
Copy link

mergify bot commented Apr 5, 2024

rebase

✅ Branch has been successfully rebased

@phlogistonjohn phlogistonjohn added the no-API This PR does not include any changes to the public API of a go-ceph package label Apr 5, 2024
@mergify mergify bot merged commit 2146f1a into ceph:master Apr 5, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-API This PR does not include any changes to the public API of a go-ceph package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants