Re: [PATCH v2 09/47] gdbsupport, filestuff, ze: temporary files

[Thiago Jung Bauermann <thiago.bauermann@linaro.org>]

Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> writes:

> @@ -544,3 +547,53 @@ read_text_file_to_string (const char *path)
>  
>    return read_remainder_of_file (file.get ());
>  }
> +
> +/* A class to keep temporary files until GDB exits (normally).  */
> +
> +struct tmpfilekeeper_s
> +{
> +  std::list<std::string> files;
> +
> +  tmpfilekeeper_s () = default;
> +  tmpfilekeeper_s (const tmpfilekeeper_s &) = delete;
> +  tmpfilekeeper_s (tmpfilekeeper_s &&) = delete;
> +
> +  ~tmpfilekeeper_s ()
> +  {
> +    for (const std::string &file : files)
> +      {
> +	const char *cfile = file.c_str ();
> +	int status = unlink (cfile);
> +	if (status != 0)
> +	  warning (_("failed to remove temporary file %s: %s"), cfile,
> +		   safe_strerror (errno));
> +      }
> +  }
> +
> +  tmpfilekeeper_s &operator= (const tmpfilekeeper_s &) = delete;
> +  tmpfilekeeper_s &operator= (tmpfilekeeper_s &&) = delete;
> +};
> +static tmpfilekeeper_s tmpfilekeeper;

There is gdb::unlinker defined in gdbsupport/gdb_unlinker.h, so
tmpfilekeeper can be an std::forward_list<gdb::unlinker> (or some other
container) and you don't need struct tmpfilekeeper_s.

But actually, gdb_create_tmpfile is used only in one place, so there's
no need for tmpfilekeepr. I have a suggestion about this in patch 10.

Also, this patch can be squashed with the next one, IMHO.

> +/* See gdbsupport/filestuff.h.  */
> +
> +gdb_file_up
> +gdb_create_tmpfile (std::string &base, int flags)
> +{
> +  std::string absbase { get_standard_temp_dir () + "/" + base };
> +  gdb::char_vector name { make_temp_filename (absbase) };
> +
> +  scoped_fd fd { gdb_mkostemp_cloexec (name.data (), O_BINARY) };
> +  if (fd.get () == -1)
> +    error (_("failed to create temporary file %s: %s"), name.data (),
> +	   safe_strerror (errno));
> +
> +  gdb_file_up file { fd.to_file ("wb") };
> +  if (file.get () == nullptr)
> +    error (_("failed to open %s: %s"), name.data (), safe_strerror (errno));
> +
> +  base = name.data ();
> +  tmpfilekeeper.files.emplace_back (base);
> +
> +  return file;
> +}
> diff --git a/gdbsupport/filestuff.h b/gdbsupport/filestuff.h
> index e2ee141d46f01bc0888a871830a5559419b63f03..6b7d1ecf9354d21e186c6f6b7aceb3aa591c4465 100644
> --- a/gdbsupport/filestuff.h
> +++ b/gdbsupport/filestuff.h
> @@ -71,6 +71,12 @@ gdb_open_cloexec (const std::string &filename, int flags,
>    return gdb_open_cloexec (filename.c_str (), flags, mode);
>  }
>  
> +/* Create a temporary file that will automatically get deleted when GDB
> +   terminates (normally).  NAME needs to be a template filename and will
> +   be modified to contain the actual template filename on return.  */

s/actual template filename/actual filename/

> +
> +extern gdb_file_up gdb_create_tmpfile (std::string &name, int flags = 0);
> +
>  /* Like 'fopen', but ensures that the returned file descriptor has the
>     close-on-exec flag set.  */

-- 
Thiago