linux-kernel - Re: [PATCH RFC 2/6] perf dso: Refactor dso_cache_

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 29 Oct 2019 11:19:15 +0200
From:   Adrian Hunter <adrian.hunter@...el.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Mathieu Poirier <mathieu.poirier@...aro.org>,
        Leo Yan <leo.yan@...aro.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC 2/6] perf dso: Refactor dso_cache__read()

On 28/10/19 5:39 PM, Jiri Olsa wrote:
> On Fri, Oct 25, 2019 at 03:59:56PM +0300, Adrian Hunter wrote:
> 
> SNIP
> 
>> +}
>>  
>> -	return ret;
>> +static struct dso_cache *dso_cache__find(struct dso *dso,
>> +					 struct machine *machine,
>> +					 u64 offset,
>> +					 ssize_t *ret)
>> +{
>> +	struct dso_cache *cache = __dso_cache__find(dso, offset);
>> +
>> +	return cache ? cache : dso_cache__populate(dso, machine, offset, ret);
>>  }
>>  
>>  static ssize_t dso_cache_read(struct dso *dso, struct machine *machine,
>>  			      u64 offset, u8 *data, ssize_t size)
>>  {
>>  	struct dso_cache *cache;
>> +	ssize_t ret = 0;
>>  
>> -	cache = dso_cache__find(dso, offset);
>> -	if (cache)
>> -		return dso_cache__memcpy(cache, offset, data, size);
>> -	else
>> -		return dso_cache__read(dso, machine, offset, data, size);
>> +	cache = dso_cache__find(dso, machine, offset, &ret);
>> +	if (!cache)
>> +		return ret;
> 
> why not use the ERR_* macros to get error through the pointer
> instead of adding extra argument?
> 

OK, here's the diff for that:

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 460330d125b6..272545624fbe 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -3,6 +3,7 @@
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/zalloc.h>
+#include <linux/err.h>
 #include <sys/time.h>
 #include <sys/resource.h>
 #include <sys/types.h>
@@ -865,30 +866,29 @@ static ssize_t file_read(struct dso *dso, struct machine *machine,
 
 static struct dso_cache *dso_cache__populate(struct dso *dso,
 					     struct machine *machine,
-					     u64 offset, ssize_t *ret)
+					     u64 offset)
 {
 	u64 cache_offset = offset & DSO__DATA_CACHE_MASK;
 	struct dso_cache *cache;
 	struct dso_cache *old;
+	ssize_t ret;
 
 	cache = zalloc(sizeof(*cache) + DSO__DATA_CACHE_SIZE);
-	if (!cache) {
-		*ret = -ENOMEM;
-		return NULL;
-	}
+	if (!cache)
+		return ERR_PTR(-ENOMEM);
 
 	if (dso->binary_type == DSO_BINARY_TYPE__BPF_PROG_INFO)
-		*ret = bpf_read(dso, cache_offset, cache->data);
+		ret = bpf_read(dso, cache_offset, cache->data);
 	else
-		*ret = file_read(dso, machine, cache_offset, cache->data);
+		ret = file_read(dso, machine, cache_offset, cache->data);
 
-	if (*ret <= 0) {
+	if (ret <= 0) {
 		free(cache);
-		return NULL;
+		return ERR_PTR(ret);
 	}
 
 	cache->offset = cache_offset;
-	cache->size   = *ret;
+	cache->size   = ret;
 
 	old = dso_cache__insert(dso, cache);
 	if (old) {
@@ -902,23 +902,20 @@ static struct dso_cache *dso_cache__populate(struct dso *dso,
 
 static struct dso_cache *dso_cache__find(struct dso *dso,
 					 struct machine *machine,
-					 u64 offset,
-					 ssize_t *ret)
+					 u64 offset)
 {
 	struct dso_cache *cache = __dso_cache__find(dso, offset);
 
-	return cache ? cache : dso_cache__populate(dso, machine, offset, ret);
+	return cache ? cache : dso_cache__populate(dso, machine, offset);
 }
 
 static ssize_t dso_cache_read(struct dso *dso, struct machine *machine,
 			      u64 offset, u8 *data, ssize_t size)
 {
-	struct dso_cache *cache;
-	ssize_t ret = 0;
+	struct dso_cache *cache = dso_cache__find(dso, machine, offset);
 
-	cache = dso_cache__find(dso, machine, offset, &ret);
-	if (!cache)
-		return ret;
+	if (IS_ERR_OR_NULL(cache))
+		return PTR_ERR(cache);
 
 	return dso_cache__memcpy(cache, offset, data, size);
 }