Message ID | 1380029714-5239-21-git-send-email-pl@kamp.de |
---|---|
State | New |
Headers | show |
On 09/24/2013 07:35 AM, Peter Lieven wrote: > Signed-off-by: Peter Lieven <pl@kamp.de> > --- > block/raw_bsd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/raw_bsd.c b/block/raw_bsd.c > index 8dc7bba..7af26ad 100644 > --- a/block/raw_bsd.c > +++ b/block/raw_bsd.c > @@ -159,6 +159,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, > Error **errp) > { > bs->sg = bs->file->sg; > + memcpy(&bs->bl, &bs->file->bl, sizeof(struct BlockLimits)); Personally, I think that sizeof(var) is more robust, because if the declaration of var is ever changed, you don't have to remember to also touch up the memcpy. As in: memcpy(&bs->bl, &bs->file->bl, sizeof(bs->bl)); But there's plenty of examples of sizeof(type) in the codebase even when a var is handy, so you are not alone, and that's not a reason for me to request a respin. On the other hand, why use memcpy() at all? bs->bl = bs->file->bl; should do the same trick, with less typing.
On Wed, Oct 02, 2013 at 11:11:05AM -0600, Eric Blake wrote: > On 09/24/2013 07:35 AM, Peter Lieven wrote: > > Signed-off-by: Peter Lieven <pl@kamp.de> > > --- > > block/raw_bsd.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/block/raw_bsd.c b/block/raw_bsd.c > > index 8dc7bba..7af26ad 100644 > > --- a/block/raw_bsd.c > > +++ b/block/raw_bsd.c > > @@ -159,6 +159,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, > > Error **errp) > > { > > bs->sg = bs->file->sg; > > + memcpy(&bs->bl, &bs->file->bl, sizeof(struct BlockLimits)); > > Personally, I think that sizeof(var) is more robust, because if the > declaration of var is ever changed, you don't have to remember to also > touch up the memcpy. As in: > > memcpy(&bs->bl, &bs->file->bl, sizeof(bs->bl)); > > But there's plenty of examples of sizeof(type) in the codebase even when > a var is handy, so you are not alone, and that's not a reason for me to > request a respin. > > On the other hand, why use memcpy() at all? > > bs->bl = bs->file->bl; > > should do the same trick, with less typing. Yes, please use struct assignment. Stefan
Am 07.10.2013 um 10:38 schrieb Stefan Hajnoczi <stefanha@gmail.com>: > On Wed, Oct 02, 2013 at 11:11:05AM -0600, Eric Blake wrote: >> On 09/24/2013 07:35 AM, Peter Lieven wrote: >>> Signed-off-by: Peter Lieven <pl@kamp.de> >>> --- >>> block/raw_bsd.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/block/raw_bsd.c b/block/raw_bsd.c >>> index 8dc7bba..7af26ad 100644 >>> --- a/block/raw_bsd.c >>> +++ b/block/raw_bsd.c >>> @@ -159,6 +159,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, >>> Error **errp) >>> { >>> bs->sg = bs->file->sg; >>> + memcpy(&bs->bl, &bs->file->bl, sizeof(struct BlockLimits)); >> >> Personally, I think that sizeof(var) is more robust, because if the >> declaration of var is ever changed, you don't have to remember to also >> touch up the memcpy. As in: >> >> memcpy(&bs->bl, &bs->file->bl, sizeof(bs->bl)); >> >> But there's plenty of examples of sizeof(type) in the codebase even when >> a var is handy, so you are not alone, and that's not a reason for me to >> request a respin. >> >> On the other hand, why use memcpy() at all? >> >> bs->bl = bs->file->bl; >> >> should do the same trick, with less typing. > > Yes, please use struct assignment. Okay, I was unsure because when looking at bdrv_move_feature_fields I found that there memcpy was used. Peter
diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 8dc7bba..7af26ad 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -159,6 +159,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { bs->sg = bs->file->sg; + memcpy(&bs->bl, &bs->file->bl, sizeof(struct BlockLimits)); return 0; }
Signed-off-by: Peter Lieven <pl@kamp.de> --- block/raw_bsd.c | 1 + 1 file changed, 1 insertion(+)