* [PATCH bpf-next 0/2] libbpf: usdt aarch64 support
@ 2022-04-08 22:52 Alan Maguire
2022-04-08 22:52 ` [PATCH bpf-next 1/2] libbpf: usdt: factor out common USDT arg handling Alan Maguire
2022-04-08 22:52 ` [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support Alan Maguire
0 siblings, 2 replies; 6+ messages in thread
From: Alan Maguire @ 2022-04-08 22:52 UTC (permalink / raw)
To: andrii, iii, ast, daniel
Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, netdev, bpf,
Alan Maguire
USDT support [1] requires architecture-specific handling for translating
from argument strings describing each probe argument to appropriate values
that can be made available to the BPF program. Determining value size,
whether it refers to a dereference (and if there is an offset from the
register value), a register value or a constant all has to be parsed
slightly differently for different architectures.
However a common representation is created for use within BPF programs
(via usdt.bpf.h), and patch 1 abstracts out the initialization of
struct usdt_arg_spec associated with the argument, rather than repeating
those steps for each architecture.
Patch 2 then adds aarch64-specific argument parsing.
[1] https://lore.kernel.org/bpf/20220404234202.331384-1-andrii@kernel.org/
Alan Maguire (2):
libbpf: usdt: factor out common USDT arg handling
libbpf: usdt aarch64 arg parsing support
tools/lib/bpf/usdt.c | 134 +++++++++++++++++++++++++++++++++------------------
1 file changed, 87 insertions(+), 47 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next 1/2] libbpf: usdt: factor out common USDT arg handling
2022-04-08 22:52 [PATCH bpf-next 0/2] libbpf: usdt aarch64 support Alan Maguire
@ 2022-04-08 22:52 ` Alan Maguire
2022-04-11 3:26 ` Andrii Nakryiko
2022-04-08 22:52 ` [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support Alan Maguire
1 sibling, 1 reply; 6+ messages in thread
From: Alan Maguire @ 2022-04-08 22:52 UTC (permalink / raw)
To: andrii, iii, ast, daniel
Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, netdev, bpf,
Alan Maguire
Common code to initialize a struct usdt_arg_spec can be factored out
from arch-specific flavours of parse_usdt_arg(); signed size,
bitshift handling etc.
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/lib/bpf/usdt.c | 90 ++++++++++++++++++++++++----------------------------
1 file changed, 41 insertions(+), 49 deletions(-)
diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
index 30c495a..0677bbd 100644
--- a/tools/lib/bpf/usdt.c
+++ b/tools/lib/bpf/usdt.c
@@ -1170,6 +1170,31 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
/* Architecture-specific logic for parsing USDT argument location specs */
+#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__)
+
+static int init_usdt_arg_spec(struct usdt_arg_spec *arg, enum usdt_arg_type arg_type, int arg_sz,
+ __u64 val_off, int reg_off)
+{
+ if (reg_off < 0)
+ return reg_off;
+ arg->arg_type = arg_type;
+ arg->val_off = val_off;
+ arg->reg_off = reg_off;
+ arg->arg_signed = arg_sz < 0;
+ if (arg_sz < 0)
+ arg_sz = -arg_sz;
+
+ switch (arg_sz) {
+ case 1: case 2: case 4: case 8:
+ arg->arg_bitshift = 64 - arg_sz * 8;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+#endif
+
#if defined(__x86_64__) || defined(__i386__)
static int calc_pt_regs_off(const char *reg_name)
@@ -1220,52 +1245,32 @@ static int calc_pt_regs_off(const char *reg_name)
static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
{
char *reg_name = NULL;
- int arg_sz, len, reg_off;
+ int arg_sz, len, ret;
long off;
if (sscanf(arg_str, " %d @ %ld ( %%%m[^)] ) %n", &arg_sz, &off, ®_name, &len) == 3) {
/* Memory dereference case, e.g., -4@-20(%rbp) */
- arg->arg_type = USDT_ARG_REG_DEREF;
- arg->val_off = off;
- reg_off = calc_pt_regs_off(reg_name);
+ ret = init_usdt_arg_spec(arg, USDT_ARG_REG_DEREF, arg_sz, off,
+ calc_pt_regs_off(reg_name));
free(reg_name);
- if (reg_off < 0)
- return reg_off;
- arg->reg_off = reg_off;
} else if (sscanf(arg_str, " %d @ %%%ms %n", &arg_sz, ®_name, &len) == 2) {
/* Register read case, e.g., -4@%eax */
- arg->arg_type = USDT_ARG_REG;
- arg->val_off = 0;
-
- reg_off = calc_pt_regs_off(reg_name);
+ ret = init_usdt_arg_spec(arg, USDT_ARG_REG, arg_sz, 0,
+ calc_pt_regs_off(reg_name));
free(reg_name);
- if (reg_off < 0)
- return reg_off;
- arg->reg_off = reg_off;
} else if (sscanf(arg_str, " %d @ $%ld %n", &arg_sz, &off, &len) == 2) {
/* Constant value case, e.g., 4@$71 */
- arg->arg_type = USDT_ARG_CONST;
- arg->val_off = off;
- arg->reg_off = 0;
+ ret = init_usdt_arg_spec(arg, USDT_ARG_CONST, arg_sz, off, 0);
} else {
pr_warn("usdt: unrecognized arg #%d spec '%s'\n", arg_num, arg_str);
return -EINVAL;
}
- arg->arg_signed = arg_sz < 0;
- if (arg_sz < 0)
- arg_sz = -arg_sz;
-
- switch (arg_sz) {
- case 1: case 2: case 4: case 8:
- arg->arg_bitshift = 64 - arg_sz * 8;
- break;
- default:
+ if (ret < 0) {
pr_warn("usdt: unsupported arg #%d (spec '%s') size: %d\n",
arg_num, arg_str, arg_sz);
- return -EINVAL;
+ return ret;
}
-
return len;
}
@@ -1276,51 +1281,38 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
{
unsigned int reg;
- int arg_sz, len;
+ int arg_sz, len, ret;
long off;
if (sscanf(arg_str, " %d @ %ld ( %%r%u ) %n", &arg_sz, &off, ®, &len) == 3) {
/* Memory dereference case, e.g., -2@-28(%r15) */
- arg->arg_type = USDT_ARG_REG_DEREF;
- arg->val_off = off;
if (reg > 15) {
pr_warn("usdt: unrecognized register '%%r%u'\n", reg);
return -EINVAL;
}
- arg->reg_off = offsetof(user_pt_regs, gprs[reg]);
+ ret = init_usdt_arg_spec(arg, USDT_ARG_REG_DEREF, arg_sz, off,
+ offsetof(user_pt_regs, gprs[reg]));
} else if (sscanf(arg_str, " %d @ %%r%u %n", &arg_sz, ®, &len) == 2) {
/* Register read case, e.g., -8@%r0 */
- arg->arg_type = USDT_ARG_REG;
- arg->val_off = 0;
if (reg > 15) {
pr_warn("usdt: unrecognized register '%%r%u'\n", reg);
return -EINVAL;
}
- arg->reg_off = offsetof(user_pt_regs, gprs[reg]);
+ ret = init_usdt_arg_spec(arg, USDT_ARG_REG, arg_sz, 0,
+ offsetof(user_pt_regs, gprs[reg]));
} else if (sscanf(arg_str, " %d @ %ld %n", &arg_sz, &off, &len) == 2) {
/* Constant value case, e.g., 4@71 */
- arg->arg_type = USDT_ARG_CONST;
- arg->val_off = off;
- arg->reg_off = 0;
+ ret = init_usdt_arg_spec(arg, USDT_ARG_CONST, arg_sz, off, 0);
} else {
pr_warn("usdt: unrecognized arg #%d spec '%s'\n", arg_num, arg_str);
return -EINVAL;
}
- arg->arg_signed = arg_sz < 0;
- if (arg_sz < 0)
- arg_sz = -arg_sz;
-
- switch (arg_sz) {
- case 1: case 2: case 4: case 8:
- arg->arg_bitshift = 64 - arg_sz * 8;
- break;
- default:
+ if (ret < 0) {
pr_warn("usdt: unsupported arg #%d (spec '%s') size: %d\n",
arg_num, arg_str, arg_sz);
- return -EINVAL;
+ return ret;
}
-
return len;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support
2022-04-08 22:52 [PATCH bpf-next 0/2] libbpf: usdt aarch64 support Alan Maguire
2022-04-08 22:52 ` [PATCH bpf-next 1/2] libbpf: usdt: factor out common USDT arg handling Alan Maguire
@ 2022-04-08 22:52 ` Alan Maguire
2022-04-11 4:11 ` Andrii Nakryiko
1 sibling, 1 reply; 6+ messages in thread
From: Alan Maguire @ 2022-04-08 22:52 UTC (permalink / raw)
To: andrii, iii, ast, daniel
Cc: kafai, songliubraving, yhs, john.fastabend, kpsingh, netdev, bpf,
Alan Maguire
Parsing of USDT arguments is architecture-specific; on aarch64 it is
relatively easy since registers used are x[0-31], sp. Format is
slightly different compared to x86_64; forms are
- "size @ [ reg[,offset] ]" for dereferences, for example
"-8 @ [ sp, 76 ]" ; " -4 @ [ sp ]"
- "size @ reg" for register values; for example
"-4@x0"
- "size @ value" for raw values; for example
"-8@1"
Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
---
tools/lib/bpf/usdt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 49 insertions(+), 1 deletion(-)
diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
index 0677bbd..6165d40 100644
--- a/tools/lib/bpf/usdt.c
+++ b/tools/lib/bpf/usdt.c
@@ -1170,7 +1170,7 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
/* Architecture-specific logic for parsing USDT argument location specs */
-#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__)
+#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__) || defined(__aarch64__)
static int init_usdt_arg_spec(struct usdt_arg_spec *arg, enum usdt_arg_type arg_type, int arg_sz,
__u64 val_off, int reg_off)
@@ -1316,6 +1316,54 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
return len;
}
+#elif defined(__aarch64__)
+
+static int calc_pt_regs_off(const char *reg_name)
+{
+ int reg_num;
+
+ if (sscanf(reg_name, "x%d", ®_num) == 1) {
+ if (reg_num >= 0 && reg_num < 31)
+ return offsetof(struct user_pt_regs, regs[reg_num]);
+ } else if (strcmp(reg_name, "sp") == 0) {
+ return offsetof(struct user_pt_regs, sp);
+ }
+ pr_warn("usdt: unrecognized register '%s'\n", reg_name);
+ return -ENOENT;
+}
+
+static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
+{
+ char *reg_name = NULL;
+ int arg_sz, len, ret;
+ long off = 0;
+
+ if (sscanf(arg_str, " %d @ \[ %m[^,], %ld ] %n", &arg_sz, ®_name, &off, &len) == 3 ||
+ sscanf(arg_str, " %d @ \[ %m[a-z0-9] ] %n", &arg_sz, ®_name, &len) == 2) {
+ /* Memory dereference case, e.g., -4@[sp, 96], -4@[sp] */
+ ret = init_usdt_arg_spec(arg, USDT_ARG_REG_DEREF, arg_sz, off,
+ calc_pt_regs_off(reg_name));
+ free(reg_name);
+ } else if (sscanf(arg_str, " %d @ %ld %n", &arg_sz, &off, &len) == 2) {
+ /* Constant value case, e.g., 4@5 */
+ ret = init_usdt_arg_spec(arg, USDT_ARG_CONST, arg_sz, off, 0);
+ } else if (sscanf(arg_str, " %d @ %ms %n", &arg_sz, ®_name, &len) == 2) {
+ /* Register read case, e.g., -8@x4 */
+ ret = init_usdt_arg_spec(arg, USDT_ARG_REG, arg_sz, 0, calc_pt_regs_off(reg_name));
+ free(reg_name);
+ } else {
+ pr_warn("usdt: unrecognized arg #%d spec '%s'\n", arg_num, arg_str);
+ return -EINVAL;
+ }
+
+ if (ret < 0) {
+ pr_warn("usdt: unsupported arg #%d (spec '%s') size: %d\n",
+ arg_num, arg_str, arg_sz);
+ return ret;
+ }
+ return len;
+}
+
#else
static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] libbpf: usdt: factor out common USDT arg handling
2022-04-08 22:52 ` [PATCH bpf-next 1/2] libbpf: usdt: factor out common USDT arg handling Alan Maguire
@ 2022-04-11 3:26 ` Andrii Nakryiko
0 siblings, 0 replies; 6+ messages in thread
From: Andrii Nakryiko @ 2022-04-11 3:26 UTC (permalink / raw)
To: Alan Maguire
Cc: Andrii Nakryiko, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song,
john fastabend, KP Singh, Networking, bpf
On Fri, Apr 8, 2022 at 3:53 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Common code to initialize a struct usdt_arg_spec can be factored out
> from arch-specific flavours of parse_usdt_arg(); signed size,
> bitshift handling etc.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
I understand that there is a bit of repetition between multiple
architectures, but I think this init_usdt_arg_spec() helper is
actually hindering understand. Can you please drop this refactoring
for now? It's just a few lines of code that you'll need to copy/paste
for argument bit shift handling, not a big deal, right?
> tools/lib/bpf/usdt.c | 90 ++++++++++++++++++++++++----------------------------
> 1 file changed, 41 insertions(+), 49 deletions(-)
>
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support
2022-04-08 22:52 ` [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support Alan Maguire
@ 2022-04-11 4:11 ` Andrii Nakryiko
2022-04-11 7:56 ` Alan Maguire
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2022-04-11 4:11 UTC (permalink / raw)
To: Alan Maguire
Cc: Andrii Nakryiko, Ilya Leoshkevich, Alexei Starovoitov,
Daniel Borkmann, Martin Lau, Song Liu, Yonghong Song,
john fastabend, KP Singh, Networking, bpf
On Fri, Apr 8, 2022 at 3:53 PM Alan Maguire <alan.maguire@oracle.com> wrote:
>
> Parsing of USDT arguments is architecture-specific; on aarch64 it is
> relatively easy since registers used are x[0-31], sp. Format is
> slightly different compared to x86_64; forms are
>
> - "size @ [ reg[,offset] ]" for dereferences, for example
> "-8 @ [ sp, 76 ]" ; " -4 @ [ sp ]"
> - "size @ reg" for register values; for example
> "-4@x0"
> - "size @ value" for raw values; for example
> "-8@1"
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> tools/lib/bpf/usdt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index 0677bbd..6165d40 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c
> @@ -1170,7 +1170,7 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
>
> /* Architecture-specific logic for parsing USDT argument location specs */
>
> -#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__)
> +#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__) || defined(__aarch64__)
>
> static int init_usdt_arg_spec(struct usdt_arg_spec *arg, enum usdt_arg_type arg_type, int arg_sz,
> __u64 val_off, int reg_off)
> @@ -1316,6 +1316,54 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
> return len;
> }
>
> +#elif defined(__aarch64__)
> +
> +static int calc_pt_regs_off(const char *reg_name)
> +{
> + int reg_num;
> +
> + if (sscanf(reg_name, "x%d", ®_num) == 1) {
> + if (reg_num >= 0 && reg_num < 31)
> + return offsetof(struct user_pt_regs, regs[reg_num]);
> + } else if (strcmp(reg_name, "sp") == 0) {
> + return offsetof(struct user_pt_regs, sp);
> + }
> + pr_warn("usdt: unrecognized register '%s'\n", reg_name);
> + return -ENOENT;
> +}
> +
> +static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
> +{
> + char *reg_name = NULL;
> + int arg_sz, len, ret;
> + long off = 0;
> +
> + if (sscanf(arg_str, " %d @ \[ %m[^,], %ld ] %n", &arg_sz, ®_name, &off, &len) == 3 ||
> + sscanf(arg_str, " %d @ \[ %m[a-z0-9] ] %n", &arg_sz, ®_name, &len) == 2) {
I'm not sure about the behavior here w.r.t. reg_name and memory
allocation. What if first sscanf() matches reg_name but fails at %ld,
will reg_name be allocated and then second sscanf() will reallocate
(and thus we'll have a memory leak).
We might have similar problems in other implementations, actually...
Either way, came here to ask to split two sscanfs into two separate
branches, so that we have a clear linear pattern. One sscanf, handle
it if successful, otherwise move on to next case.
Also a question about [a-z0-9] for register in one case and [^,] in
another. Should the first one be [a-z0-9] as well?
> + /* Memory dereference case, e.g., -4@[sp, 96], -4@[sp] */
> + ret = init_usdt_arg_spec(arg, USDT_ARG_REG_DEREF, arg_sz, off,
> + calc_pt_regs_off(reg_name));
> + free(reg_name);
> + } else if (sscanf(arg_str, " %d @ %ld %n", &arg_sz, &off, &len) == 2) {
> + /* Constant value case, e.g., 4@5 */
> + ret = init_usdt_arg_spec(arg, USDT_ARG_CONST, arg_sz, off, 0);
> + } else if (sscanf(arg_str, " %d @ %ms %n", &arg_sz, ®_name, &len) == 2) {
> + /* Register read case, e.g., -8@x4 */
> + ret = init_usdt_arg_spec(arg, USDT_ARG_REG, arg_sz, 0, calc_pt_regs_off(reg_name));
> + free(reg_name);
> + } else {
> + pr_warn("usdt: unrecognized arg #%d spec '%s'\n", arg_num, arg_str);
> + return -EINVAL;
> + }
> +
> + if (ret < 0) {
> + pr_warn("usdt: unsupported arg #%d (spec '%s') size: %d\n",
> + arg_num, arg_str, arg_sz);
> + return ret;
> + }
> + return len;
> +}
> +
> #else
>
> static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support
2022-04-11 4:11 ` Andrii Nakryiko
@ 2022-04-11 7:56 ` Alan Maguire
0 siblings, 0 replies; 6+ messages in thread
From: Alan Maguire @ 2022-04-11 7:56 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Alan Maguire, Andrii Nakryiko, Ilya Leoshkevich,
Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, john fastabend, KP Singh, Networking, bpf
On Mon, 11 Apr 2022, Andrii Nakryiko wrote:
> On Fri, Apr 8, 2022 at 3:53 PM Alan Maguire <alan.maguire@oracle.com> wrote:
> >
> > Parsing of USDT arguments is architecture-specific; on aarch64 it is
> > relatively easy since registers used are x[0-31], sp. Format is
> > slightly different compared to x86_64; forms are
> >
> > - "size @ [ reg[,offset] ]" for dereferences, for example
> > "-8 @ [ sp, 76 ]" ; " -4 @ [ sp ]"
> > - "size @ reg" for register values; for example
> > "-4@x0"
> > - "size @ value" for raw values; for example
> > "-8@1"
> >
> > Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> > ---
> > tools/lib/bpf/usdt.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 49 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> > index 0677bbd..6165d40 100644
> > --- a/tools/lib/bpf/usdt.c
> > +++ b/tools/lib/bpf/usdt.c
> > @@ -1170,7 +1170,7 @@ static int parse_usdt_spec(struct usdt_spec *spec, const struct usdt_note *note,
> >
> > /* Architecture-specific logic for parsing USDT argument location specs */
> >
> > -#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__)
> > +#if defined(__x86_64__) || defined(__i386__) || defined(__s390x__) || defined(__aarch64__)
> >
> > static int init_usdt_arg_spec(struct usdt_arg_spec *arg, enum usdt_arg_type arg_type, int arg_sz,
> > __u64 val_off, int reg_off)
> > @@ -1316,6 +1316,54 @@ static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec
> > return len;
> > }
> >
> > +#elif defined(__aarch64__)
> > +
> > +static int calc_pt_regs_off(const char *reg_name)
> > +{
> > + int reg_num;
> > +
> > + if (sscanf(reg_name, "x%d", ®_num) == 1) {
> > + if (reg_num >= 0 && reg_num < 31)
> > + return offsetof(struct user_pt_regs, regs[reg_num]);
> > + } else if (strcmp(reg_name, "sp") == 0) {
> > + return offsetof(struct user_pt_regs, sp);
> > + }
> > + pr_warn("usdt: unrecognized register '%s'\n", reg_name);
> > + return -ENOENT;
> > +}
> > +
> > +static int parse_usdt_arg(const char *arg_str, int arg_num, struct usdt_arg_spec *arg)
> > +{
> > + char *reg_name = NULL;
> > + int arg_sz, len, ret;
> > + long off = 0;
> > +
> > + if (sscanf(arg_str, " %d @ \[ %m[^,], %ld ] %n", &arg_sz, ®_name, &off, &len) == 3 ||
> > + sscanf(arg_str, " %d @ \[ %m[a-z0-9] ] %n", &arg_sz, ®_name, &len) == 2) {
>
> I'm not sure about the behavior here w.r.t. reg_name and memory
> allocation. What if first sscanf() matches reg_name but fails at %ld,
> will reg_name be allocated and then second sscanf() will reallocate
> (and thus we'll have a memory leak).
>
> We might have similar problems in other implementations, actually...
>
> Either way, came here to ask to split two sscanfs into two separate
> branches, so that we have a clear linear pattern. One sscanf, handle
> it if successful, otherwise move on to next case.
>
good point; I'll separate the sscanfs into branches for v2.
> Also a question about [a-z0-9] for register in one case and [^,] in
> another. Should the first one be [a-z0-9] as well?
>
probably no harm, yep.
I'll drop the refactoring patch too; I was a bit worried I'd break
Ilya's s390 code anyhow.
Thanks!
Alan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-11 8:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-08 22:52 [PATCH bpf-next 0/2] libbpf: usdt aarch64 support Alan Maguire
2022-04-08 22:52 ` [PATCH bpf-next 1/2] libbpf: usdt: factor out common USDT arg handling Alan Maguire
2022-04-11 3:26 ` Andrii Nakryiko
2022-04-08 22:52 ` [PATCH bpf-next 2/2] libbpf: usdt aarch64 arg parsing support Alan Maguire
2022-04-11 4:11 ` Andrii Nakryiko
2022-04-11 7:56 ` Alan Maguire
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.