From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43977) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZNilr-0003gB-Fv for qemu-devel@nongnu.org; Fri, 07 Aug 2015 10:37:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZNilq-0008Ku-HL for qemu-devel@nongnu.org; Fri, 07 Aug 2015 10:37:19 -0400 References: <1438807969-9723-1-git-send-email-mreitz@redhat.com> <1438807969-9723-2-git-send-email-mreitz@redhat.com> <55C2BFEA.5090800@cn.fujitsu.com> From: Max Reitz Message-ID: <55C4C292.9050904@redhat.com> Date: Fri, 7 Aug 2015 16:37:06 +0200 MIME-Version: 1.0 In-Reply-To: <55C2BFEA.5090800@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/5] block: Change bdrv_get_encrypted_filename() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 06.08.2015 04:01, Wen Congyang wrote: > On 08/06/2015 04:52 AM, Max Reitz wrote: >> Instead of returning a pointer to the filename, copy it into a buffer >> specified by the caller. >> >> Signed-off-by: Max Reitz >> --- >> block.c | 24 +++++++++++++++++------- >> include/block/block.h | 2 +- >> monitor.c | 4 +++- >> 3 files changed, 21 insertions(+), 9 deletions(-) >> >> diff --git a/block.c b/block.c >> index d088ee0..e7dd2f1 100644 >> --- a/block.c >> +++ b/block.c >> @@ -2760,10 +2760,12 @@ void bdrv_add_key(BlockDriverState *bs, const char *key, Error **errp) >> } >> } else { >> if (bdrv_key_required(bs)) { >> + char enc_filename[PATH_MAX]; > > I think it's better to use g_malloc() here. > >> + bdrv_get_encrypted_filename(bs, enc_filename, sizeof(enc_filename)); >> error_set(errp, ERROR_CLASS_DEVICE_ENCRYPTED, >> "'%s' (%s) is encrypted", >> bdrv_get_device_or_node_name(bs), >> - bdrv_get_encrypted_filename(bs)); >> + enc_filename); >> } >> } >> } >> @@ -2980,14 +2982,22 @@ bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs) >> return false; >> } >> >> -const char *bdrv_get_encrypted_filename(BlockDriverState *bs) >> +char *bdrv_get_encrypted_filename(BlockDriverState *bs, char *dest, size_t sz) >> { >> - if (bs->backing_hd && bs->backing_hd->encrypted) >> - return bs->backing_file; >> - else if (bs->encrypted) >> - return bs->filename; >> - else >> + if (sz > INT_MAX) { >> + sz = INT_MAX; >> + } >> + >> + if (bs->backing_hd && bs->backing_hd->encrypted) { >> + pstrcpy(dest, sz, bs->backing_file); >> + return dest; >> + } else if (bs->encrypted) { >> + pstrcpy(dest, sz, bs->filename); >> + return dest; >> + } else { >> + dest[0] = '\0'; >> return NULL; >> + } >> } >> >> void bdrv_get_backing_filename(BlockDriverState *bs, >> diff --git a/include/block/block.h b/include/block/block.h >> index 37916f7..a78e4f1 100644 >> --- a/include/block/block.h >> +++ b/include/block/block.h >> @@ -430,7 +430,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs, >> int64_t *cluster_sector_num, >> int *cluster_nb_sectors); >> >> -const char *bdrv_get_encrypted_filename(BlockDriverState *bs); >> +char *bdrv_get_encrypted_filename(BlockDriverState *bs, char *dest, size_t sz); >> void bdrv_get_backing_filename(BlockDriverState *bs, >> char *filename, int filename_size); >> void bdrv_get_full_backing_filename(BlockDriverState *bs, >> diff --git a/monitor.c b/monitor.c >> index aeea2b5..cfdf781 100644 >> --- a/monitor.c >> +++ b/monitor.c >> @@ -5292,10 +5292,12 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs, >> BlockCompletionFunc *completion_cb, >> void *opaque) >> { >> + char enc_filename[PATH_MAX]; > > same too. You're right. I'll change it (in every patch of this series that pattern appears in). Thanks for reviewing! Max > > Thanks > Wen Congyang > >> int err; >> >> + bdrv_get_encrypted_filename(bs, enc_filename, sizeof(enc_filename)); >> monitor_printf(mon, "%s (%s) is encrypted.\n", bdrv_get_device_name(bs), >> - bdrv_get_encrypted_filename(bs)); >> + enc_filename); >> >> mon->password_completion_cb = completion_cb; >> mon->password_opaque = opaque; >> >