| Message ID | 20210127085752.120571-6-aik@ozlabs.ru |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Compile with -Wextra | expand |
On 27/01/2021 09.57, Alexey Kardashevskiy wrote: > -Wextra enables a bunch of rather useful checks which this fixes. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > lib/libveth/veth.h | 2 +- > lib/libveth/veth.c | 8 ++++++-- > 2 files changed, 7 insertions(+), 3 deletions(-) > > diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h > index 23af0eab6211..6a1cb4cb5790 100644 > --- a/lib/libveth/veth.h > +++ b/lib/libveth/veth.h > @@ -16,7 +16,7 @@ > #include <stdint.h> > #include <netdriver.h> > > -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, int reg_len); > +extern net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char *reg, unsigned reg_len); > extern void libveth_close(net_driver_t *driver); > extern int libveth_read(char *buf, int len, net_driver_t *driver); > extern int libveth_write(char *buf, int len, net_driver_t *driver); > diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c > index 748730854035..a8e19ba41764 100644 > --- a/lib/libveth/veth.c > +++ b/lib/libveth/veth.c > @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) > return 0; > } > > -static int veth_receive(char *f_buffer_pc, int f_len_i, net_driver_t *driver) > +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, net_driver_t *driver) > { > int packet = 0; > > @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int f_len_i, net_driver_t *driver) > return f_len_i; > } > > -net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, int reg_len) > +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char *reg, unsigned reg_len) > { > net_driver_t *driver; > > + if (reg_len != sizeof(uint32_t)) { > + printf("vio reg must 1 cell long\n"); > + return NULL; > + } > driver = SLOF_alloc_mem(sizeof(*driver)); > if (!driver) { > printf("Unable to allocate veth driver\n"); > Is this patch necessary at all? mac_len is only used to compare with 8, and reg_len should not matter since you've finally added -Wno-unused-param ... so I think you could simply drop this again? Thomas
On 29/01/2021 01:31, Thomas Huth wrote: > On 27/01/2021 09.57, Alexey Kardashevskiy wrote: >> -Wextra enables a bunch of rather useful checks which this fixes. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> lib/libveth/veth.h | 2 +- >> lib/libveth/veth.c | 8 ++++++-- >> 2 files changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h >> index 23af0eab6211..6a1cb4cb5790 100644 >> --- a/lib/libveth/veth.h >> +++ b/lib/libveth/veth.h >> @@ -16,7 +16,7 @@ >> #include <stdint.h> >> #include <netdriver.h> >> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, char >> *reg, int reg_len); >> +extern net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, >> char *reg, unsigned reg_len); >> extern void libveth_close(net_driver_t *driver); >> extern int libveth_read(char *buf, int len, net_driver_t *driver); >> extern int libveth_write(char *buf, int len, net_driver_t *driver); >> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c >> index 748730854035..a8e19ba41764 100644 >> --- a/lib/libveth/veth.c >> +++ b/lib/libveth/veth.c >> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) >> return 0; >> } >> -static int veth_receive(char *f_buffer_pc, int f_len_i, net_driver_t >> *driver) >> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, >> net_driver_t *driver) >> { >> int packet = 0; >> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int >> f_len_i, net_driver_t *driver) >> return f_len_i; >> } >> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, >> int reg_len) >> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char >> *reg, unsigned reg_len) >> { >> net_driver_t *driver; >> + if (reg_len != sizeof(uint32_t)) { >> + printf("vio reg must 1 cell long\n"); >> + return NULL; >> + } >> driver = SLOF_alloc_mem(sizeof(*driver)); >> if (!driver) { >> printf("Unable to allocate veth driver\n"); >> > > Is this patch necessary at all? Did you mean "this hunk"? > mac_len is only used to compare with 8, Then we need to get rid of mac_len which I do not mind but not in this patchset. > and reg_len should not matter since you've finally added > -Wno-unused-param ... so I think you could simply drop this again? This number comes from SLOF, I'd rather know if there is garbage. Thanks,
On 29/01/2021 03.04, Alexey Kardashevskiy wrote: > > > On 29/01/2021 01:31, Thomas Huth wrote: >> On 27/01/2021 09.57, Alexey Kardashevskiy wrote: >>> -Wextra enables a bunch of rather useful checks which this fixes. >>> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>> --- >>> lib/libveth/veth.h | 2 +- >>> lib/libveth/veth.c | 8 ++++++-- >>> 2 files changed, 7 insertions(+), 3 deletions(-) >>> >>> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h >>> index 23af0eab6211..6a1cb4cb5790 100644 >>> --- a/lib/libveth/veth.h >>> +++ b/lib/libveth/veth.h >>> @@ -16,7 +16,7 @@ >>> #include <stdint.h> >>> #include <netdriver.h> >>> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, char >>> *reg, int reg_len); >>> +extern net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char >>> *reg, unsigned reg_len); >>> extern void libveth_close(net_driver_t *driver); >>> extern int libveth_read(char *buf, int len, net_driver_t *driver); >>> extern int libveth_write(char *buf, int len, net_driver_t *driver); >>> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c >>> index 748730854035..a8e19ba41764 100644 >>> --- a/lib/libveth/veth.c >>> +++ b/lib/libveth/veth.c >>> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) >>> return 0; >>> } >>> -static int veth_receive(char *f_buffer_pc, int f_len_i, net_driver_t >>> *driver) >>> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, >>> net_driver_t *driver) >>> { >>> int packet = 0; >>> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int >>> f_len_i, net_driver_t *driver) >>> return f_len_i; >>> } >>> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, int >>> reg_len) >>> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char *reg, >>> unsigned reg_len) >>> { >>> net_driver_t *driver; >>> + if (reg_len != sizeof(uint32_t)) { >>> + printf("vio reg must 1 cell long\n"); >>> + return NULL; >>> + } >>> driver = SLOF_alloc_mem(sizeof(*driver)); >>> if (!driver) { >>> printf("Unable to allocate veth driver\n"); >>> >> >> Is this patch necessary at all? > > Did you mean "this hunk"? Yes, I meant hunk. When I compile with -Wextra -Wno-unused-parameter, I only get a warning in veth_receive(), but not in libveth_open(). Thomas
On 29/01/2021 17:36, Thomas Huth wrote: > On 29/01/2021 03.04, Alexey Kardashevskiy wrote: >> >> >> On 29/01/2021 01:31, Thomas Huth wrote: >>> On 27/01/2021 09.57, Alexey Kardashevskiy wrote: >>>> -Wextra enables a bunch of rather useful checks which this fixes. >>>> >>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>> --- >>>> lib/libveth/veth.h | 2 +- >>>> lib/libveth/veth.c | 8 ++++++-- >>>> 2 files changed, 7 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h >>>> index 23af0eab6211..6a1cb4cb5790 100644 >>>> --- a/lib/libveth/veth.h >>>> +++ b/lib/libveth/veth.h >>>> @@ -16,7 +16,7 @@ >>>> #include <stdint.h> >>>> #include <netdriver.h> >>>> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, char >>>> *reg, int reg_len); >>>> +extern net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, >>>> char *reg, unsigned reg_len); >>>> extern void libveth_close(net_driver_t *driver); >>>> extern int libveth_read(char *buf, int len, net_driver_t *driver); >>>> extern int libveth_write(char *buf, int len, net_driver_t *driver); >>>> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c >>>> index 748730854035..a8e19ba41764 100644 >>>> --- a/lib/libveth/veth.c >>>> +++ b/lib/libveth/veth.c >>>> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) >>>> return 0; >>>> } >>>> -static int veth_receive(char *f_buffer_pc, int f_len_i, >>>> net_driver_t *driver) >>>> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, >>>> net_driver_t *driver) >>>> { >>>> int packet = 0; >>>> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int >>>> f_len_i, net_driver_t *driver) >>>> return f_len_i; >>>> } >>>> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, >>>> int reg_len) >>>> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char >>>> *reg, unsigned reg_len) >>>> { >>>> net_driver_t *driver; >>>> + if (reg_len != sizeof(uint32_t)) { >>>> + printf("vio reg must 1 cell long\n"); >>>> + return NULL; >>>> + } >>>> driver = SLOF_alloc_mem(sizeof(*driver)); >>>> if (!driver) { >>>> printf("Unable to allocate veth driver\n"); >>>> >>> >>> Is this patch necessary at all? >> >> Did you mean "this hunk"? > > Yes, I meant hunk. When I compile with -Wextra -Wno-unused-parameter, I > only get a warning in veth_receive(), but not in libveth_open(). And? Make this a separate patch? The patch make it compile with "-Wextra" and without -Wno-unused-parameter, is that bad?
On 01/02/2021 03.42, Alexey Kardashevskiy wrote: > > > On 29/01/2021 17:36, Thomas Huth wrote: >> On 29/01/2021 03.04, Alexey Kardashevskiy wrote: >>> >>> >>> On 29/01/2021 01:31, Thomas Huth wrote: >>>> On 27/01/2021 09.57, Alexey Kardashevskiy wrote: >>>>> -Wextra enables a bunch of rather useful checks which this fixes. >>>>> >>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>>> --- >>>>> lib/libveth/veth.h | 2 +- >>>>> lib/libveth/veth.c | 8 ++++++-- >>>>> 2 files changed, 7 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h >>>>> index 23af0eab6211..6a1cb4cb5790 100644 >>>>> --- a/lib/libveth/veth.h >>>>> +++ b/lib/libveth/veth.h >>>>> @@ -16,7 +16,7 @@ >>>>> #include <stdint.h> >>>>> #include <netdriver.h> >>>>> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, char >>>>> *reg, int reg_len); >>>>> +extern net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, >>>>> char *reg, unsigned reg_len); >>>>> extern void libveth_close(net_driver_t *driver); >>>>> extern int libveth_read(char *buf, int len, net_driver_t *driver); >>>>> extern int libveth_write(char *buf, int len, net_driver_t *driver); >>>>> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c >>>>> index 748730854035..a8e19ba41764 100644 >>>>> --- a/lib/libveth/veth.c >>>>> +++ b/lib/libveth/veth.c >>>>> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) >>>>> return 0; >>>>> } >>>>> -static int veth_receive(char *f_buffer_pc, int f_len_i, net_driver_t >>>>> *driver) >>>>> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, >>>>> net_driver_t *driver) >>>>> { >>>>> int packet = 0; >>>>> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int >>>>> f_len_i, net_driver_t *driver) >>>>> return f_len_i; >>>>> } >>>>> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, int >>>>> reg_len) >>>>> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char >>>>> *reg, unsigned reg_len) >>>>> { >>>>> net_driver_t *driver; >>>>> + if (reg_len != sizeof(uint32_t)) { >>>>> + printf("vio reg must 1 cell long\n"); >>>>> + return NULL; >>>>> + } >>>>> driver = SLOF_alloc_mem(sizeof(*driver)); >>>>> if (!driver) { >>>>> printf("Unable to allocate veth driver\n"); >>>>> >>>> >>>> Is this patch necessary at all? >>> >>> Did you mean "this hunk"? >> >> Yes, I meant hunk. When I compile with -Wextra -Wno-unused-parameter, I >> only get a warning in veth_receive(), but not in libveth_open(). > > > And? Make this a separate patch? The patch make it compile with "-Wextra" > and without -Wno-unused-parameter, is that bad? It just doesn't "match" to what you said in the cover letter / do in the last patch. I think I'd be fine with this if you'd mentioned it at least in the cover letter that you address the "unused-parameter" warnings, too. Thomas
On 01/02/2021 17:09, Thomas Huth wrote: > On 01/02/2021 03.42, Alexey Kardashevskiy wrote: >> >> >> On 29/01/2021 17:36, Thomas Huth wrote: >>> On 29/01/2021 03.04, Alexey Kardashevskiy wrote: >>>> >>>> >>>> On 29/01/2021 01:31, Thomas Huth wrote: >>>>> On 27/01/2021 09.57, Alexey Kardashevskiy wrote: >>>>>> -Wextra enables a bunch of rather useful checks which this fixes. >>>>>> >>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>>>> --- >>>>>> lib/libveth/veth.h | 2 +- >>>>>> lib/libveth/veth.c | 8 ++++++-- >>>>>> 2 files changed, 7 insertions(+), 3 deletions(-) >>>>>> >>>>>> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h >>>>>> index 23af0eab6211..6a1cb4cb5790 100644 >>>>>> --- a/lib/libveth/veth.h >>>>>> +++ b/lib/libveth/veth.h >>>>>> @@ -16,7 +16,7 @@ >>>>>> #include <stdint.h> >>>>>> #include <netdriver.h> >>>>>> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, >>>>>> char *reg, int reg_len); >>>>>> +extern net_driver_t *libveth_open(char *mac_addr, unsigned >>>>>> mac_len, char *reg, unsigned reg_len); >>>>>> extern void libveth_close(net_driver_t *driver); >>>>>> extern int libveth_read(char *buf, int len, net_driver_t *driver); >>>>>> extern int libveth_write(char *buf, int len, net_driver_t *driver); >>>>>> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c >>>>>> index 748730854035..a8e19ba41764 100644 >>>>>> --- a/lib/libveth/veth.c >>>>>> +++ b/lib/libveth/veth.c >>>>>> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) >>>>>> return 0; >>>>>> } >>>>>> -static int veth_receive(char *f_buffer_pc, int f_len_i, >>>>>> net_driver_t *driver) >>>>>> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, >>>>>> net_driver_t *driver) >>>>>> { >>>>>> int packet = 0; >>>>>> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int >>>>>> f_len_i, net_driver_t *driver) >>>>>> return f_len_i; >>>>>> } >>>>>> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char >>>>>> *reg, int reg_len) >>>>>> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char >>>>>> *reg, unsigned reg_len) >>>>>> { >>>>>> net_driver_t *driver; >>>>>> + if (reg_len != sizeof(uint32_t)) { >>>>>> + printf("vio reg must 1 cell long\n"); >>>>>> + return NULL; >>>>>> + } >>>>>> driver = SLOF_alloc_mem(sizeof(*driver)); >>>>>> if (!driver) { >>>>>> printf("Unable to allocate veth driver\n"); >>>>>> >>>>> >>>>> Is this patch necessary at all? >>>> >>>> Did you mean "this hunk"? >>> >>> Yes, I meant hunk. When I compile with -Wextra -Wno-unused-parameter, >>> I only get a warning in veth_receive(), but not in libveth_open(). >> >> >> And? Make this a separate patch? The patch make it compile with >> "-Wextra" and without -Wno-unused-parameter, is that bad? > > It just doesn't "match" to what you said in the cover letter / do in the > last patch. I think I'd be fine with this if you'd mentioned it at least > in the cover letter that you address the "unused-parameter" warnings, too. Fair enough :) I just do not take cover letter too seriously as they do not even make it to the patchworks, not to mention git history. I'll update it though in the next spin. Thanks,
On Mon, 1 Feb 2021 17:12:03 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > > On 01/02/2021 17:09, Thomas Huth wrote: > > On 01/02/2021 03.42, Alexey Kardashevskiy wrote: > >> > >> > >> On 29/01/2021 17:36, Thomas Huth wrote: > >>> On 29/01/2021 03.04, Alexey Kardashevskiy wrote: > >>>> > >>>> > >>>> On 29/01/2021 01:31, Thomas Huth wrote: > >>>>> On 27/01/2021 09.57, Alexey Kardashevskiy wrote: > >>>>>> -Wextra enables a bunch of rather useful checks which this fixes. > >>>>>> > >>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >>>>>> --- > >>>>>> lib/libveth/veth.h | 2 +- > >>>>>> lib/libveth/veth.c | 8 ++++++-- > >>>>>> 2 files changed, 7 insertions(+), 3 deletions(-) > >>>>>> > >>>>>> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h > >>>>>> index 23af0eab6211..6a1cb4cb5790 100644 > >>>>>> --- a/lib/libveth/veth.h > >>>>>> +++ b/lib/libveth/veth.h > >>>>>> @@ -16,7 +16,7 @@ > >>>>>> #include <stdint.h> > >>>>>> #include <netdriver.h> > >>>>>> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, > >>>>>> char *reg, int reg_len); > >>>>>> +extern net_driver_t *libveth_open(char *mac_addr, unsigned > >>>>>> mac_len, char *reg, unsigned reg_len); > >>>>>> extern void libveth_close(net_driver_t *driver); > >>>>>> extern int libveth_read(char *buf, int len, net_driver_t *driver); > >>>>>> extern int libveth_write(char *buf, int len, net_driver_t *driver); > >>>>>> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c > >>>>>> index 748730854035..a8e19ba41764 100644 > >>>>>> --- a/lib/libveth/veth.c > >>>>>> +++ b/lib/libveth/veth.c > >>>>>> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) > >>>>>> return 0; > >>>>>> } > >>>>>> -static int veth_receive(char *f_buffer_pc, int f_len_i, > >>>>>> net_driver_t *driver) > >>>>>> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, > >>>>>> net_driver_t *driver) > >>>>>> { > >>>>>> int packet = 0; > >>>>>> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int > >>>>>> f_len_i, net_driver_t *driver) > >>>>>> return f_len_i; > >>>>>> } > >>>>>> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char > >>>>>> *reg, int reg_len) > >>>>>> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char > >>>>>> *reg, unsigned reg_len) > >>>>>> { > >>>>>> net_driver_t *driver; > >>>>>> + if (reg_len != sizeof(uint32_t)) { > >>>>>> + printf("vio reg must 1 cell long\n"); > >>>>>> + return NULL; > >>>>>> + } > >>>>>> driver = SLOF_alloc_mem(sizeof(*driver)); > >>>>>> if (!driver) { > >>>>>> printf("Unable to allocate veth driver\n"); > >>>>>> > >>>>> > >>>>> Is this patch necessary at all? > >>>> > >>>> Did you mean "this hunk"? > >>> > >>> Yes, I meant hunk. When I compile with -Wextra -Wno-unused-parameter, > >>> I only get a warning in veth_receive(), but not in libveth_open(). > >> > >> > >> And? Make this a separate patch? The patch make it compile with > >> "-Wextra" and without -Wno-unused-parameter, is that bad? > > > > It just doesn't "match" to what you said in the cover letter / do in the > > last patch. I think I'd be fine with this if you'd mentioned it at least > > in the cover letter that you address the "unused-parameter" warnings, too. > > Fair enough :) I just do not take cover letter too seriously as they do > not even make it to the patchworks, not to mention git history. I'll They actually make it to the patchworks :) http://patchwork.ozlabs.org/project/slof/cover/20210127085752.120571-1-aik@ozlabs.ru/ > update it though in the next spin. Thanks, > > > >
On 01/02/2021 19:50, Greg Kurz wrote: > On Mon, 1 Feb 2021 17:12:03 +1100 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > >> >> >> On 01/02/2021 17:09, Thomas Huth wrote: >>> On 01/02/2021 03.42, Alexey Kardashevskiy wrote: >>>> >>>> >>>> On 29/01/2021 17:36, Thomas Huth wrote: >>>>> On 29/01/2021 03.04, Alexey Kardashevskiy wrote: >>>>>> >>>>>> >>>>>> On 29/01/2021 01:31, Thomas Huth wrote: >>>>>>> On 27/01/2021 09.57, Alexey Kardashevskiy wrote: >>>>>>>> -Wextra enables a bunch of rather useful checks which this fixes. >>>>>>>> >>>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>>>>>> --- >>>>>>>> lib/libveth/veth.h | 2 +- >>>>>>>> lib/libveth/veth.c | 8 ++++++-- >>>>>>>> 2 files changed, 7 insertions(+), 3 deletions(-) >>>>>>>> >>>>>>>> diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h >>>>>>>> index 23af0eab6211..6a1cb4cb5790 100644 >>>>>>>> --- a/lib/libveth/veth.h >>>>>>>> +++ b/lib/libveth/veth.h >>>>>>>> @@ -16,7 +16,7 @@ >>>>>>>> #include <stdint.h> >>>>>>>> #include <netdriver.h> >>>>>>>> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, >>>>>>>> char *reg, int reg_len); >>>>>>>> +extern net_driver_t *libveth_open(char *mac_addr, unsigned >>>>>>>> mac_len, char *reg, unsigned reg_len); >>>>>>>> extern void libveth_close(net_driver_t *driver); >>>>>>>> extern int libveth_read(char *buf, int len, net_driver_t *driver); >>>>>>>> extern int libveth_write(char *buf, int len, net_driver_t *driver); >>>>>>>> diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c >>>>>>>> index 748730854035..a8e19ba41764 100644 >>>>>>>> --- a/lib/libveth/veth.c >>>>>>>> +++ b/lib/libveth/veth.c >>>>>>>> @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) >>>>>>>> return 0; >>>>>>>> } >>>>>>>> -static int veth_receive(char *f_buffer_pc, int f_len_i, >>>>>>>> net_driver_t *driver) >>>>>>>> +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, >>>>>>>> net_driver_t *driver) >>>>>>>> { >>>>>>>> int packet = 0; >>>>>>>> @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int >>>>>>>> f_len_i, net_driver_t *driver) >>>>>>>> return f_len_i; >>>>>>>> } >>>>>>>> -net_driver_t *libveth_open(char *mac_addr, int mac_len, char >>>>>>>> *reg, int reg_len) >>>>>>>> +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char >>>>>>>> *reg, unsigned reg_len) >>>>>>>> { >>>>>>>> net_driver_t *driver; >>>>>>>> + if (reg_len != sizeof(uint32_t)) { >>>>>>>> + printf("vio reg must 1 cell long\n"); >>>>>>>> + return NULL; >>>>>>>> + } >>>>>>>> driver = SLOF_alloc_mem(sizeof(*driver)); >>>>>>>> if (!driver) { >>>>>>>> printf("Unable to allocate veth driver\n"); >>>>>>>> >>>>>>> >>>>>>> Is this patch necessary at all? >>>>>> >>>>>> Did you mean "this hunk"? >>>>> >>>>> Yes, I meant hunk. When I compile with -Wextra -Wno-unused-parameter, >>>>> I only get a warning in veth_receive(), but not in libveth_open(). >>>> >>>> >>>> And? Make this a separate patch? The patch make it compile with >>>> "-Wextra" and without -Wno-unused-parameter, is that bad? >>> >>> It just doesn't "match" to what you said in the cover letter / do in the >>> last patch. I think I'd be fine with this if you'd mentioned it at least >>> in the cover letter that you address the "unused-parameter" warnings, too. >> >> Fair enough :) I just do not take cover letter too seriously as they do >> not even make it to the patchworks, not to mention git history. I'll > > They actually make it to the patchworks :) > > http://patchwork.ozlabs.org/project/slof/cover/20210127085752.120571-1-aik@ozlabs.ru/ how did you come up with this url? i cannot find a button for it in the ui. > >> update it though in the next spin. Thanks, >> >> >> >> >
On Mon, 1 Feb 2021 19:53:59 +1100 Alexey Kardashevskiy <aik@ozlabs.ru> wrote: [...] > >> > >> Fair enough :) I just do not take cover letter too seriously as they do > >> not even make it to the patchworks, not to mention git history. I'll > > > > They actually make it to the patchworks :) > > > > http://patchwork.ozlabs.org/project/slof/cover/20210127085752.120571-1-aik@ozlabs.ru/ > > how did you come up with this url? i cannot find a button for it in the ui. > Go to any patch of this series, e.g. http://patchwork.ozlabs.org/project/slof/patch/20210127085752.120571-2-aik@ozlabs.ru/ There's a line with: Series Compile with -Wextra | expand ^^ Click here > > > > >> update it though in the next spin. Thanks, > >> > >> > >> > >> > > >
On 01/02/2021 20:07, Greg Kurz wrote: > On Mon, 1 Feb 2021 19:53:59 +1100 > Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > > [...] > >>>> >>>> Fair enough :) I just do not take cover letter too seriously as they do >>>> not even make it to the patchworks, not to mention git history. I'll >>> >>> They actually make it to the patchworks :) >>> >>> http://patchwork.ozlabs.org/project/slof/cover/20210127085752.120571-1-aik@ozlabs.ru/ >> >> how did you come up with this url? i cannot find a button for it in the ui. >> > > Go to any patch of this series, e.g. > > http://patchwork.ozlabs.org/project/slof/patch/20210127085752.120571-2-aik@ozlabs.ru/ > > There's a line with: > > Series Compile with -Wextra | expand > ^^ > Click here oh. awesome ui :) I looked there but somehow missed it :-/ thanks! > >> >>> >>>> update it though in the next spin. Thanks, >>>> >>>> >>>> >>>> >>> >> >
On Mon, Feb 01, 2021 at 08:15:28PM +1100, Alexey Kardashevskiy wrote: > On 01/02/2021 20:07, Greg Kurz wrote: > >On Mon, 1 Feb 2021 19:53:59 +1100 > >Alexey Kardashevskiy <aik@ozlabs.ru> wrote: > >>how did you come up with this url? i cannot find a button for it in the > >>ui. > > > >Go to any patch of this series, e.g. > > > >http://patchwork.ozlabs.org/project/slof/patch/20210127085752.120571-2-aik@ozlabs.ru/ > > > >There's a line with: > > > >Series Compile with -Wextra | expand > > ^^ > > Click here > > oh. awesome ui :) I looked there but somehow missed it :-/ thanks! Heh. Or you can go to any message in the series, look at the headers, and paste the in-reply-to message-id into the url. "Some people find that easier", or perhaps they just didn't know the other way. I'm not admitting either way :-) Thanks, Segher
diff --git a/lib/libveth/veth.h b/lib/libveth/veth.h index 23af0eab6211..6a1cb4cb5790 100644 --- a/lib/libveth/veth.h +++ b/lib/libveth/veth.h @@ -16,7 +16,7 @@ #include <stdint.h> #include <netdriver.h> -extern net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, int reg_len); +extern net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char *reg, unsigned reg_len); extern void libveth_close(net_driver_t *driver); extern int libveth_read(char *buf, int len, net_driver_t *driver); extern int libveth_write(char *buf, int len, net_driver_t *driver); diff --git a/lib/libveth/veth.c b/lib/libveth/veth.c index 748730854035..a8e19ba41764 100644 --- a/lib/libveth/veth.c +++ b/lib/libveth/veth.c @@ -164,7 +164,7 @@ static int veth_term(net_driver_t *driver) return 0; } -static int veth_receive(char *f_buffer_pc, int f_len_i, net_driver_t *driver) +static int veth_receive(char *f_buffer_pc, unsigned f_len_i, net_driver_t *driver) { int packet = 0; @@ -223,10 +223,14 @@ static int veth_xmit(char *f_buffer_pc, int f_len_i, net_driver_t *driver) return f_len_i; } -net_driver_t *libveth_open(char *mac_addr, int mac_len, char *reg, int reg_len) +net_driver_t *libveth_open(char *mac_addr, unsigned mac_len, char *reg, unsigned reg_len) { net_driver_t *driver; + if (reg_len != sizeof(uint32_t)) { + printf("vio reg must 1 cell long\n"); + return NULL; + } driver = SLOF_alloc_mem(sizeof(*driver)); if (!driver) { printf("Unable to allocate veth driver\n");
-Wextra enables a bunch of rather useful checks which this fixes. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- lib/libveth/veth.h | 2 +- lib/libveth/veth.c | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-)