On Mon, Oct 10, 2022 at 12:05 PM Bin Meng <bin.m...@windriver.com> wrote: > > At present there are two callers of get_tmp_filename() and they are > inconsistent. > > One does: > > /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */ > char *tmp_filename = g_malloc0(PATH_MAX + 1); > ... > ret = get_tmp_filename(tmp_filename, PATH_MAX + 1); > > while the other does: > > s->qcow_filename = g_malloc(PATH_MAX); > ret = get_tmp_filename(s->qcow_filename, PATH_MAX); > > As we can see different 'size' arguments are passed. There are also > platform specific implementations inside the function, and the use > of snprintf is really undesirable. > > The function name is also misleading. It creates a temporary file, > not just a filename. > > Refactor this routine by changing its name and signature to: > > char *create_tmp_file(Error **errp) > > and use g_get_tmp_dir() / g_mkstemp() for a consistent implementation. > > While we are here, add some comments to mention that /var/tmp is > preferred over /tmp on non-win32 hosts. > > Signed-off-by: Bin Meng <bin.m...@windriver.com> > --- > > Changes in v6: > - use g_mkstemp() and stick to use /var/tmp for non-win32 hosts > > Changes in v5: > - minor change in the commit message > - add some notes in the function comment block > - add g_autofree for tmp_filename > > Changes in v4: > - Rename the function to create_tmp_file() and take "Error **errp" as > a parameter, so that callers can pass errp all the way down to this > routine. > - Commit message updated to reflect the latest change > > Changes in v3: > - Do not use errno directly, instead still let get_tmp_filename() return > a negative number to indicate error > > Changes in v2: > - Use g_autofree and g_steal_pointer > > include/block/block_int-common.h | 2 +- > block.c | 56 +++++++++++++++++--------------- > block/vvfat.c | 7 ++-- > 3 files changed, 34 insertions(+), 31 deletions(-) >
Any comments?