Re: [PATCH v4 19/27] wrapper.c: wrapper to open a file, fprintf then close

[Eric Sunshine]

On Sat, Mar 1, 2014 at 12:11 PM, Torsten Bögershausen tbo...@web.de wrote:
 On 2014-03-01 13.12, Nguyễn Thái Ngọc Duy wrote:
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  cache.h   |  2 ++
  wrapper.c | 31 +++
  2 files changed, 33 insertions(+)

 diff --git a/cache.h b/cache.h
 index 98b5dd3..99b86d9 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -1239,6 +1239,8 @@ static inline ssize_t write_str_in_full(int fd, const 
 char *str)
  {
   return write_in_full(fd, str, strlen(str));
  }
 +__attribute__((format (printf,3,4)))
 +extern int write_file(const char *path, int fatal, const char *fmt, ...);

  /* pager.c */
  extern void setup_pager(void);
 diff --git a/wrapper.c b/wrapper.c
 index 0cc5636..5ced50d 100644
 --- a/wrapper.c
 +++ b/wrapper.c
 @@ -455,3 +455,34 @@ struct passwd *xgetpwuid_self(void)
   errno ? strerror(errno) : _(no such user));
   return pw;
  }
 +
 +int write_file(const char *path, int fatal, const char *fmt, ...)
 +{
 + struct strbuf sb = STRBUF_INIT;
 + int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0666);
 + va_list params;
 + if (fd  0) {

Micro nit atop Torsten's micro nit:

It is 3% easier to understand the code if the check for open() failure
immediately follows the open() attempt:

va_list params;
int fd = open(...);
if (fd  0) {

 + if (fatal)
 + die_errno(_(could not open %s for writing), path);
 + return -1;
 + }
 + va_start(params, fmt);
 + strbuf_vaddf(sb, fmt, params);
 + va_end(params);
 + if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
 + int err = errno;
 + close(fd);
 + errno = err;
 + strbuf_release(sb);
 Micro nit:
 Today we now what strbuf_release() is doing, but if we ever change the
 implementation, it is 3% safer to keep err a little bit longer like this:
 + strbuf_release(sb);
 + errno = err;
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 19/27] wrapper.c: wrapper to open a file, fprintf then close

[Torsten Bögershausen]

On 2014-03-01 13.12, Nguyễn Thái Ngọc Duy wrote:
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  cache.h   |  2 ++
  wrapper.c | 31 +++
  2 files changed, 33 insertions(+)
 
 diff --git a/cache.h b/cache.h
 index 98b5dd3..99b86d9 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -1239,6 +1239,8 @@ static inline ssize_t write_str_in_full(int fd, const 
 char *str)
  {
   return write_in_full(fd, str, strlen(str));
  }
 +__attribute__((format (printf,3,4)))
 +extern int write_file(const char *path, int fatal, const char *fmt, ...);
  
  /* pager.c */
  extern void setup_pager(void);
 diff --git a/wrapper.c b/wrapper.c
 index 0cc5636..5ced50d 100644
 --- a/wrapper.c
 +++ b/wrapper.c
 @@ -455,3 +455,34 @@ struct passwd *xgetpwuid_self(void)
   errno ? strerror(errno) : _(no such user));
   return pw;
  }
 +
 +int write_file(const char *path, int fatal, const char *fmt, ...)
 +{
 + struct strbuf sb = STRBUF_INIT;
 + int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0666);
 + va_list params;
 + if (fd  0) {
 + if (fatal)
 + die_errno(_(could not open %s for writing), path);
 + return -1;
 + }
 + va_start(params, fmt);
 + strbuf_vaddf(sb, fmt, params);
 + va_end(params);
 + if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
 + int err = errno;
 + close(fd);
 + errno = err;
 + strbuf_release(sb);
Micro nit:
Today we now what strbuf_release() is doing, but if we ever change the
implementation, it is 3% safer to keep err a little bit longer like this:
 + strbuf_release(sb);
 + errno = err;

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html