Hi Daniel,
> Use stat before open the file to find out if the
> file already exists. Only if file exists and
> the header is invalid trigger connman_error
> ---
> src/stats.c | 24 +++++++++++++++---------
> 1 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/src/stats.c b/src/stats.c
> index 08f3dd2..9e14021 100644
> --- a/src/stats.c
> +++ b/src/stats.c
> @@ -250,11 +250,12 @@ static int stats_open_file(struct connman_service
> *service,
> struct stats_file *file,
> connman_bool_t roaming)
> {
> - struct stat stat;
> + struct stat buf;
please don't. The common declaration here is this:
struct stat st;
And buf is the worst variable name you could have picked ;)
> char *name;
> int err;
> size_t size;
> struct stats_file_header *hdr;
> + connman_bool_t new_file = FALSE;
>
> if (roaming == FALSE) {
> name = g_strdup_printf("%s/%s.data", STORAGEDIR,
> @@ -265,6 +266,13 @@ static int stats_open_file(struct connman_service
> *service,
> }
>
> file->name = name;
> +
> + err = stat(file->name, &buf);
> + if (err < 0) {
> + /* according documentation the only possible error is ENOENT */
> + new_file = TRUE;
> + }
> +
> file->fd = open(name, O_RDWR | O_CREAT, 0644);
>
> if (file->fd == -1) {
I prefer these checks like this btw.:
if (fd < 0)
error...
> @@ -275,14 +283,11 @@ static int stats_open_file(struct connman_service
> *service,
>
> file->max_len = STATS_MAX_FILE_SIZE;
>
> - err = fstat(file->fd, &stat);
> - if (err < 0)
> - return -errno;
> -
> - if (stat.st_size < sysconf(_SC_PAGESIZE))
> + if (buf.st_size < sysconf(_SC_PAGESIZE))
> size = sysconf(_SC_PAGESIZE);
> else
> - size = (size_t)stat.st_size;
> + size = (size_t)buf.st_size;
> +
What is this cast for. I don't really like casts like this.
> err = stats_file_remap(file, size);
> if (err < 0)
> @@ -290,12 +295,13 @@ static int stats_open_file(struct connman_service
> *service,
>
> hdr = get_hdr(file);
>
> - if (hdr->magic != MAGIC ||
> + if (new_file == TRUE || hdr->magic != MAGIC ||
> hdr->begin < sizeof(struct stats_file_header) ||
> hdr->end < sizeof(struct stats_file_header) ||
> hdr->begin > file->len ||
> hdr->end > file->len) {
> - connman_warn("invalid file header for %s", file->name);
> + if (new_file == FALSE)
> + connman_warn("invalid file header for %s", file->name);
First check for new_file == TRUE and now for new_file == FALSE. That
logic seems a bit too complicated for me. Makes my brain hurt ;)
Regards
Marcel
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
