Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1650268 > unrolled thread

Re: [PATCH v2 06/13] perf header: add struct feat_fd for write

Started byJiri Olsa <jolsa@redhat.com>
First post2017-05-25 10:10 +0200
Last post2017-06-05 04:00 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 06/13] perf header: add struct feat_fd for write Jiri Olsa <jolsa@redhat.com> - 2017-05-25 10:10 +0200
    Re: [PATCH v2 06/13] perf header: add struct feat_fd for write David Carrillo-Cisneros <davidcc@google.com> - 2017-06-05 04:00 +0200

#1650268 — Re: [PATCH v2 06/13] perf header: add struct feat_fd for write

FromJiri Olsa <jolsa@redhat.com>
Date2017-05-25 10:10 +0200
SubjectRe: [PATCH v2 06/13] perf header: add struct feat_fd for write
Message-ID<tKW0i-5FH-3@gated-at.bofh.it>
On Tue, May 23, 2017 at 12:48:46AM -0700, David Carrillo-Cisneros wrote:

SNIP

> @@ -2211,18 +2214,18 @@ static int do_write_feat(int fd, struct perf_header *h, int type,
>  		if (!feat_ops[type].write)
>  			return -1;
>  
> -		(*p)->offset = lseek(fd, 0, SEEK_CUR);
> +		(*p)->offset = lseek(fd->fd, 0, SEEK_CUR);
>  
> -		err = feat_ops[type].write(fd, h, evlist);
> +		err = feat_ops[type].write(fd, evlist);
>  		if (err < 0) {
>  			pr_debug("failed to write feature %s\n", feat_ops[type].name);
>  
>  			/* undo anything written */
> -			lseek(fd, (*p)->offset, SEEK_SET);
> +			lseek(fd->fd, (*p)->offset, SEEK_SET);
>  
>  			return -1;
>  		}
> -		(*p)->size = lseek(fd, 0, SEEK_CUR) - (*p)->offset;
> +		(*p)->size = lseek(fd->fd, 0, SEEK_CUR) - (*p)->offset;
>  		(*p)++;
>  	}
>  	return ret;
> @@ -2232,12 +2235,18 @@ static int perf_header__adds_write(struct perf_header *header,
>  				   struct perf_evlist *evlist, int fd)
>  {
>  	int nr_sections;
> +	struct feat_fd fdd;
>  	struct perf_file_section *feat_sec, *p;
>  	int sec_size;
>  	u64 sec_start;
>  	int feat;
>  	int err;
>  
> +	fdd = (struct feat_fd){
> +		.fd  = fd,
> +		.ph = header,
> +	};

could you unite the naming for struct feat_fd variables?

it's slightly confusing, especialy when you keep the 'fd'
name in some cases.. ff? ;-)

jirka

[toc] | [next] | [standalone]


#1657213

FromDavid Carrillo-Cisneros <davidcc@google.com>
Date2017-06-05 04:00 +0200
Message-ID<tOPtf-4zu-3@gated-at.bofh.it>
In reply to#1650268
On Thu, May 25, 2017 at 1:07 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Tue, May 23, 2017 at 12:48:46AM -0700, David Carrillo-Cisneros wrote:
>
> SNIP
>
>> @@ -2211,18 +2214,18 @@ static int do_write_feat(int fd, struct perf_header *h, int type,
>>               if (!feat_ops[type].write)
>>                       return -1;
>>
>> -             (*p)->offset = lseek(fd, 0, SEEK_CUR);
>> +             (*p)->offset = lseek(fd->fd, 0, SEEK_CUR);
>>
>> -             err = feat_ops[type].write(fd, h, evlist);
>> +             err = feat_ops[type].write(fd, evlist);
>>               if (err < 0) {
>>                       pr_debug("failed to write feature %s\n", feat_ops[type].name);
>>
>>                       /* undo anything written */
>> -                     lseek(fd, (*p)->offset, SEEK_SET);
>> +                     lseek(fd->fd, (*p)->offset, SEEK_SET);
>>
>>                       return -1;
>>               }
>> -             (*p)->size = lseek(fd, 0, SEEK_CUR) - (*p)->offset;
>> +             (*p)->size = lseek(fd->fd, 0, SEEK_CUR) - (*p)->offset;
>>               (*p)++;
>>       }
>>       return ret;
>> @@ -2232,12 +2235,18 @@ static int perf_header__adds_write(struct perf_header *header,
>>                                  struct perf_evlist *evlist, int fd)
>>  {
>>       int nr_sections;
>> +     struct feat_fd fdd;
>>       struct perf_file_section *feat_sec, *p;
>>       int sec_size;
>>       u64 sec_start;
>>       int feat;
>>       int err;
>>
>> +     fdd = (struct feat_fd){
>> +             .fd  = fd,
>> +             .ph = header,
>> +     };
>
> could you unite the naming for struct feat_fd variables?
>
> it's slightly confusing, especialy when you keep the 'fd'
> name in some cases.. ff? ;-)
>
Thank you for the review :) . I'll do the rename.

> jirka

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web