Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
Hi, On Tue, Apr 16, 2019 at 08:51:52AM +, Victor Toso wrote: > Hi, > > On Mon, Apr 15, 2019 at 03:42:05PM +0300, Yuri Benditovich wrote: > > IIUC, what you call 'simpler' is: > > - making unneeded changes in several files (instead of one) > > - in the next patch remove these changes completely > > > > Did I miss something? > > Current patch series changes spice_usbutil_libusb_strerror(), > then drops its usage and then a new patch to remove > spice_usbutil_libusb_strerror() is needed. > > My suggestion was to remove spice_usbutil_libusb_strerror() as > first thing, because you can replace that with > libusb_error_name(). > > From the repository POV, this is nicer IMHO. Yes, you would need > to rebase your branch. > > Note that this is a friendly review, if you disagree feel free to > say so and why. I'm trying to make my rationale clear above so > you can just clarify why my suggestion is bad, if you think so. > > I'll be looking further to the other patches Today, sorry for > taking some time on it. Yes, taking more time that I anticipated. The whole USB stack is new to me and that makes a good opportunity to understand more and more things which delays a bit my review here. Replying here because I did a FIXUP patch that removes the function as suggested and the following up patches are almost untouched, see the branch yuri-ml-usb-backend-layer [0] for the patch + rebase. [0] https://gitlab.freedesktop.org/victortoso/spice-gtk/commits/yuri-ml-usb-backend-layer One other thing that bothered me was the #ifdef USE_POLKIT on channel-usbredir which I proposed to remove. I've rebased your code on top of that as well, needs a small FIXUP patch it seems. [1] https://gitlab.freedesktop.org/victortoso/spice-gtk/commits/yuri-usb-backend-on-polkit-branch I'll be providing you better feedback later this week. Cheers, > Cheers, > Victor > > > On Mon, Apr 15, 2019 at 3:18 PM Christophe Fergeau > > wrote: > > > > > > On Thu, Apr 11, 2019 at 12:37:17PM +, Victor Toso wrote: > > > > Hi, > > > > > > > > On Thu, Apr 11, 2019 at 02:57:21PM +0300, Yuri Benditovich wrote: > > > > > On Thu, Apr 11, 2019 at 12:35 PM Victor Toso > > > > > wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote: > > > > > > > libusb has libusb_error_name procedure that returns name > > > > > > > for any error that libusb may return, so we do not need > > > > > > > to analyze error values by ourselves. > > > > > > > > > > > > > > Signed-off-by: Yuri Benditovich > > > > > > > > > > > > Before applying the series: > > > > > > > > > > > > (master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/ > > > > > > src/win-usb-dev.c:116:const char *errstr = > > > > > > spice_usbutil_libusb_strerror(rc); > > > > > > src/win-usb-dev.c:173:const char *errstr = > > > > > > spice_usbutil_libusb_strerror(rc); > > > > > > src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc); > > > > > > src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum > > > > > > libusb_error error_code) > > > > > > src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum > > > > > > libusb_error error_code); > > > > > > src/usb-device-manager.c:284:const char *desc = > > > > > > spice_usbutil_libusb_strerror(rc); > > > > > > src/usb-device-manager.c:311:const char *desc = > > > > > > spice_usbutil_libusb_strerror(rc); > > > > > > src/usb-device-manager.c:733:errstr = > > > > > > spice_usbutil_libusb_strerror(errcode); > > > > > > src/usb-device-manager.c:1071:const char *desc = > > > > > > spice_usbutil_libusb_strerror(rc); > > > > > > > > > > > > After applying the series: > > > > > > (yuri-usb-b-layers-v1 5f87d90d) $ grepi > > > > > > "spice_usbutil_libusb_strerror" src/ > > > > > > (yuri-usb-b-layers-v1 5f87d90d) $ > > > > > > > > > > > > So, I think it makes sense to use this patch to drop this > > > > > > function and always use libusb_error_name() instead, agree? > > > > > > > > > > Finally, this series drops this functions and uses libusb_error_name. > > > > > > > > Yes, > > > > > > > > > It was possible to drop this function in the first patch, but > > > > > this would not make too much sense ( as all these new calls to > > > > > libusb_error_name() would be removed due to isolation of > > > > > libusb). > > > > > > > > For me it makes sense because I know that this function can be > > > > dropped now even if later patches would change the code path of > > > > callers of spice_usbutil_libusb_strerror/libusb_error_name again. > > > > > > > > That is, removing this function as first patch would introduce no > > > > regression and cleanup the code a bit. One patch less in the > > > > queue and could be merged before the others ;) > > > > > > Yep, makes sense to me too to start by making the code simpler, and > > > merging the preparatory patches early. > > > > > > Christophe >
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
Hi, On Mon, Apr 15, 2019 at 03:42:05PM +0300, Yuri Benditovich wrote: > IIUC, what you call 'simpler' is: > - making unneeded changes in several files (instead of one) > - in the next patch remove these changes completely > > Did I miss something? Current patch series changes spice_usbutil_libusb_strerror(), then drops its usage and then a new patch to remove spice_usbutil_libusb_strerror() is needed. My suggestion was to remove spice_usbutil_libusb_strerror() as first thing, because you can replace that with libusb_error_name(). From the repository POV, this is nicer IMHO. Yes, you would need to rebase your branch. Note that this is a friendly review, if you disagree feel free to say so and why. I'm trying to make my rationale clear above so you can just clarify why my suggestion is bad, if you think so. I'll be looking further to the other patches Today, sorry for taking some time on it. Cheers, Victor > On Mon, Apr 15, 2019 at 3:18 PM Christophe Fergeau > wrote: > > > > On Thu, Apr 11, 2019 at 12:37:17PM +, Victor Toso wrote: > > > Hi, > > > > > > On Thu, Apr 11, 2019 at 02:57:21PM +0300, Yuri Benditovich wrote: > > > > On Thu, Apr 11, 2019 at 12:35 PM Victor Toso > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote: > > > > > > libusb has libusb_error_name procedure that returns name > > > > > > for any error that libusb may return, so we do not need > > > > > > to analyze error values by ourselves. > > > > > > > > > > > > Signed-off-by: Yuri Benditovich > > > > > > > > > > Before applying the series: > > > > > > > > > > (master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/ > > > > > src/win-usb-dev.c:116:const char *errstr = > > > > > spice_usbutil_libusb_strerror(rc); > > > > > src/win-usb-dev.c:173:const char *errstr = > > > > > spice_usbutil_libusb_strerror(rc); > > > > > src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc); > > > > > src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum > > > > > libusb_error error_code) > > > > > src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum > > > > > libusb_error error_code); > > > > > src/usb-device-manager.c:284:const char *desc = > > > > > spice_usbutil_libusb_strerror(rc); > > > > > src/usb-device-manager.c:311:const char *desc = > > > > > spice_usbutil_libusb_strerror(rc); > > > > > src/usb-device-manager.c:733:errstr = > > > > > spice_usbutil_libusb_strerror(errcode); > > > > > src/usb-device-manager.c:1071:const char *desc = > > > > > spice_usbutil_libusb_strerror(rc); > > > > > > > > > > After applying the series: > > > > > (yuri-usb-b-layers-v1 5f87d90d) $ grepi > > > > > "spice_usbutil_libusb_strerror" src/ > > > > > (yuri-usb-b-layers-v1 5f87d90d) $ > > > > > > > > > > So, I think it makes sense to use this patch to drop this > > > > > function and always use libusb_error_name() instead, agree? > > > > > > > > Finally, this series drops this functions and uses libusb_error_name. > > > > > > Yes, > > > > > > > It was possible to drop this function in the first patch, but > > > > this would not make too much sense ( as all these new calls to > > > > libusb_error_name() would be removed due to isolation of > > > > libusb). > > > > > > For me it makes sense because I know that this function can be > > > dropped now even if later patches would change the code path of > > > callers of spice_usbutil_libusb_strerror/libusb_error_name again. > > > > > > That is, removing this function as first patch would introduce no > > > regression and cleanup the code a bit. One patch less in the > > > queue and could be merged before the others ;) > > > > Yep, makes sense to me too to start by making the code simpler, and > > merging the preparatory patches early. > > > > Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
On Mon, Apr 15, 2019 at 03:42:05PM +0300, Yuri Benditovich wrote: > IIUC, what you call 'simpler' is: > - making unneeded changes in several files (instead of one) > - in the next patch remove these changes completely > > Did I miss something? Ah I did not look at the next patches :) Since you intentionally keep this wrapper because it's going to be shortlived, I'd explain it in the commit log so that it's clear that it's intentional. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
IIUC, what you call 'simpler' is: - making unneeded changes in several files (instead of one) - in the next patch remove these changes completely Did I miss something? On Mon, Apr 15, 2019 at 3:18 PM Christophe Fergeau wrote: > > On Thu, Apr 11, 2019 at 12:37:17PM +, Victor Toso wrote: > > Hi, > > > > On Thu, Apr 11, 2019 at 02:57:21PM +0300, Yuri Benditovich wrote: > > > On Thu, Apr 11, 2019 at 12:35 PM Victor Toso > > > wrote: > > > > > > > > Hi, > > > > > > > > On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote: > > > > > libusb has libusb_error_name procedure that returns name > > > > > for any error that libusb may return, so we do not need > > > > > to analyze error values by ourselves. > > > > > > > > > > Signed-off-by: Yuri Benditovich > > > > > > > > Before applying the series: > > > > > > > > (master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/ > > > > src/win-usb-dev.c:116:const char *errstr = > > > > spice_usbutil_libusb_strerror(rc); > > > > src/win-usb-dev.c:173:const char *errstr = > > > > spice_usbutil_libusb_strerror(rc); > > > > src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc); > > > > src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum > > > > libusb_error error_code) > > > > src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum > > > > libusb_error error_code); > > > > src/usb-device-manager.c:284:const char *desc = > > > > spice_usbutil_libusb_strerror(rc); > > > > src/usb-device-manager.c:311:const char *desc = > > > > spice_usbutil_libusb_strerror(rc); > > > > src/usb-device-manager.c:733:errstr = > > > > spice_usbutil_libusb_strerror(errcode); > > > > src/usb-device-manager.c:1071:const char *desc = > > > > spice_usbutil_libusb_strerror(rc); > > > > > > > > After applying the series: > > > > (yuri-usb-b-layers-v1 5f87d90d) $ grepi "spice_usbutil_libusb_strerror" > > > > src/ > > > > (yuri-usb-b-layers-v1 5f87d90d) $ > > > > > > > > So, I think it makes sense to use this patch to drop this > > > > function and always use libusb_error_name() instead, agree? > > > > > > Finally, this series drops this functions and uses libusb_error_name. > > > > Yes, > > > > > It was possible to drop this function in the first patch, but > > > this would not make too much sense ( as all these new calls to > > > libusb_error_name() would be removed due to isolation of > > > libusb). > > > > For me it makes sense because I know that this function can be > > dropped now even if later patches would change the code path of > > callers of spice_usbutil_libusb_strerror/libusb_error_name again. > > > > That is, removing this function as first patch would introduce no > > regression and cleanup the code a bit. One patch less in the > > queue and could be merged before the others ;) > > Yep, makes sense to me too to start by making the code simpler, and > merging the preparatory patches early. > > Christophe ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
On Thu, Apr 11, 2019 at 12:37:17PM +, Victor Toso wrote: > Hi, > > On Thu, Apr 11, 2019 at 02:57:21PM +0300, Yuri Benditovich wrote: > > On Thu, Apr 11, 2019 at 12:35 PM Victor Toso wrote: > > > > > > Hi, > > > > > > On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote: > > > > libusb has libusb_error_name procedure that returns name > > > > for any error that libusb may return, so we do not need > > > > to analyze error values by ourselves. > > > > > > > > Signed-off-by: Yuri Benditovich > > > > > > Before applying the series: > > > > > > (master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/ > > > src/win-usb-dev.c:116:const char *errstr = > > > spice_usbutil_libusb_strerror(rc); > > > src/win-usb-dev.c:173:const char *errstr = > > > spice_usbutil_libusb_strerror(rc); > > > src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc); > > > src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum > > > libusb_error error_code) > > > src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum > > > libusb_error error_code); > > > src/usb-device-manager.c:284:const char *desc = > > > spice_usbutil_libusb_strerror(rc); > > > src/usb-device-manager.c:311:const char *desc = > > > spice_usbutil_libusb_strerror(rc); > > > src/usb-device-manager.c:733:errstr = > > > spice_usbutil_libusb_strerror(errcode); > > > src/usb-device-manager.c:1071:const char *desc = > > > spice_usbutil_libusb_strerror(rc); > > > > > > After applying the series: > > > (yuri-usb-b-layers-v1 5f87d90d) $ grepi "spice_usbutil_libusb_strerror" > > > src/ > > > (yuri-usb-b-layers-v1 5f87d90d) $ > > > > > > So, I think it makes sense to use this patch to drop this > > > function and always use libusb_error_name() instead, agree? > > > > Finally, this series drops this functions and uses libusb_error_name. > > Yes, > > > It was possible to drop this function in the first patch, but > > this would not make too much sense ( as all these new calls to > > libusb_error_name() would be removed due to isolation of > > libusb). > > For me it makes sense because I know that this function can be > dropped now even if later patches would change the code path of > callers of spice_usbutil_libusb_strerror/libusb_error_name again. > > That is, removing this function as first patch would introduce no > regression and cleanup the code a bit. One patch less in the > queue and could be merged before the others ;) Yep, makes sense to me too to start by making the code simpler, and merging the preparatory patches early. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
Hi,
On Thu, Apr 11, 2019 at 02:57:21PM +0300, Yuri Benditovich wrote:
> On Thu, Apr 11, 2019 at 12:35 PM Victor Toso wrote:
> >
> > Hi,
> >
> > On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote:
> > > libusb has libusb_error_name procedure that returns name
> > > for any error that libusb may return, so we do not need
> > > to analyze error values by ourselves.
> > >
> > > Signed-off-by: Yuri Benditovich
> >
> > Before applying the series:
> >
> > (master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/
> > src/win-usb-dev.c:116:const char *errstr =
> > spice_usbutil_libusb_strerror(rc);
> > src/win-usb-dev.c:173:const char *errstr =
> > spice_usbutil_libusb_strerror(rc);
> > src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc);
> > src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum
> > libusb_error error_code)
> > src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum
> > libusb_error error_code);
> > src/usb-device-manager.c:284:const char *desc =
> > spice_usbutil_libusb_strerror(rc);
> > src/usb-device-manager.c:311:const char *desc =
> > spice_usbutil_libusb_strerror(rc);
> > src/usb-device-manager.c:733:errstr =
> > spice_usbutil_libusb_strerror(errcode);
> > src/usb-device-manager.c:1071:const char *desc =
> > spice_usbutil_libusb_strerror(rc);
> >
> > After applying the series:
> > (yuri-usb-b-layers-v1 5f87d90d) $ grepi "spice_usbutil_libusb_strerror" src/
> > (yuri-usb-b-layers-v1 5f87d90d) $
> >
> > So, I think it makes sense to use this patch to drop this
> > function and always use libusb_error_name() instead, agree?
>
> Finally, this series drops this functions and uses libusb_error_name.
Yes,
> It was possible to drop this function in the first patch, but
> this would not make too much sense ( as all these new calls to
> libusb_error_name() would be removed due to isolation of
> libusb).
For me it makes sense because I know that this function can be
dropped now even if later patches would change the code path of
callers of spice_usbutil_libusb_strerror/libusb_error_name again.
That is, removing this function as first patch would introduce no
regression and cleanup the code a bit. One patch less in the
queue and could be merged before the others ;)
I'm on the middle of different task but I plan to study a bit
2nd/3rd patch Tomorrow/next week.
Cheers,
> > I checked that all above have include
> >
> > I'll also be looking into your other patches but it might take
> > some time to get familiar with the changes :)
> >
> > Cheers,
> >
> > > ---
> > > src/usbutil.c | 32 +---
> > > 1 file changed, 1 insertion(+), 31 deletions(-)
> > >
> > > diff --git a/src/usbutil.c b/src/usbutil.c
> > > index e96ab11..4aa6ef7 100644
> > > --- a/src/usbutil.c
> > > +++ b/src/usbutil.c
> > > @@ -61,37 +61,7 @@ static usb_vendor_info *usbids_vendor_info = NULL;
> > > G_GNUC_INTERNAL
> > > const char *spice_usbutil_libusb_strerror(enum libusb_error error_code)
> > > {
> > > -switch (error_code) {
> > > -case LIBUSB_SUCCESS:
> > > -return "Success";
> > > -case LIBUSB_ERROR_IO:
> > > -return "Input/output error";
> > > -case LIBUSB_ERROR_INVALID_PARAM:
> > > -return "Invalid parameter";
> > > -case LIBUSB_ERROR_ACCESS:
> > > -return "Access denied (insufficient permissions)";
> > > -case LIBUSB_ERROR_NO_DEVICE:
> > > -return "No such device (it may have been disconnected)";
> > > -case LIBUSB_ERROR_NOT_FOUND:
> > > -return "Entity not found";
> > > -case LIBUSB_ERROR_BUSY:
> > > -return "Resource busy";
> > > -case LIBUSB_ERROR_TIMEOUT:
> > > -return "Operation timed out";
> > > -case LIBUSB_ERROR_OVERFLOW:
> > > -return "Overflow";
> > > -case LIBUSB_ERROR_PIPE:
> > > -return "Pipe error";
> > > -case LIBUSB_ERROR_INTERRUPTED:
> > > -return "System call interrupted (perhaps due to signal)";
> > > -case LIBUSB_ERROR_NO_MEM:
> > > -return "Insufficient memory";
> > > -case LIBUSB_ERROR_NOT_SUPPORTED:
> > > -return "Operation not supported or unimplemented on this
> > > platform";
> > > -case LIBUSB_ERROR_OTHER:
> > > -return "Other error";
> > > -}
> > > -return "Unknown error";
> > > +return libusb_error_name(error_code);
> > > }
> > >
> > > #ifdef __linux__
> > > --
> > > 2.17.1
> > >
> > > ___
> > > Spice-devel mailing list
> > > Spice-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
On Thu, Apr 11, 2019 at 12:35 PM Victor Toso wrote:
>
> Hi,
>
> On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote:
> > libusb has libusb_error_name procedure that returns name
> > for any error that libusb may return, so we do not need
> > to analyze error values by ourselves.
> >
> > Signed-off-by: Yuri Benditovich
>
> Before applying the series:
>
> (master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/
> src/win-usb-dev.c:116:const char *errstr =
> spice_usbutil_libusb_strerror(rc);
> src/win-usb-dev.c:173:const char *errstr =
> spice_usbutil_libusb_strerror(rc);
> src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc);
> src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum libusb_error
> error_code)
> src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum libusb_error
> error_code);
> src/usb-device-manager.c:284:const char *desc =
> spice_usbutil_libusb_strerror(rc);
> src/usb-device-manager.c:311:const char *desc =
> spice_usbutil_libusb_strerror(rc);
> src/usb-device-manager.c:733:errstr =
> spice_usbutil_libusb_strerror(errcode);
> src/usb-device-manager.c:1071:const char *desc =
> spice_usbutil_libusb_strerror(rc);
>
> After applying the series:
> (yuri-usb-b-layers-v1 5f87d90d) $ grepi "spice_usbutil_libusb_strerror" src/
> (yuri-usb-b-layers-v1 5f87d90d) $
>
> So, I think it makes sense to use this patch to drop this
> function and always use libusb_error_name() instead, agree?
Finally, this series drops this functions and uses libusb_error_name.
It was possible to drop this function in the first patch, but this
would not make too much sense
( as all these new calls to libusb_error_name() would be removed due
to isolation of libusb).
>
> I checked that all above have include
>
> I'll also be looking into your other patches but it might take
> some time to get familiar with the changes :)
>
> Cheers,
>
> > ---
> > src/usbutil.c | 32 +---
> > 1 file changed, 1 insertion(+), 31 deletions(-)
> >
> > diff --git a/src/usbutil.c b/src/usbutil.c
> > index e96ab11..4aa6ef7 100644
> > --- a/src/usbutil.c
> > +++ b/src/usbutil.c
> > @@ -61,37 +61,7 @@ static usb_vendor_info *usbids_vendor_info = NULL;
> > G_GNUC_INTERNAL
> > const char *spice_usbutil_libusb_strerror(enum libusb_error error_code)
> > {
> > -switch (error_code) {
> > -case LIBUSB_SUCCESS:
> > -return "Success";
> > -case LIBUSB_ERROR_IO:
> > -return "Input/output error";
> > -case LIBUSB_ERROR_INVALID_PARAM:
> > -return "Invalid parameter";
> > -case LIBUSB_ERROR_ACCESS:
> > -return "Access denied (insufficient permissions)";
> > -case LIBUSB_ERROR_NO_DEVICE:
> > -return "No such device (it may have been disconnected)";
> > -case LIBUSB_ERROR_NOT_FOUND:
> > -return "Entity not found";
> > -case LIBUSB_ERROR_BUSY:
> > -return "Resource busy";
> > -case LIBUSB_ERROR_TIMEOUT:
> > -return "Operation timed out";
> > -case LIBUSB_ERROR_OVERFLOW:
> > -return "Overflow";
> > -case LIBUSB_ERROR_PIPE:
> > -return "Pipe error";
> > -case LIBUSB_ERROR_INTERRUPTED:
> > -return "System call interrupted (perhaps due to signal)";
> > -case LIBUSB_ERROR_NO_MEM:
> > -return "Insufficient memory";
> > -case LIBUSB_ERROR_NOT_SUPPORTED:
> > -return "Operation not supported or unimplemented on this platform";
> > -case LIBUSB_ERROR_OTHER:
> > -return "Other error";
> > -}
> > -return "Unknown error";
> > +return libusb_error_name(error_code);
> > }
> >
> > #ifdef __linux__
> > --
> > 2.17.1
> >
> > ___
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
Hi,
On Wed, Apr 10, 2019 at 10:31:37PM +0300, Yuri Benditovich wrote:
> libusb has libusb_error_name procedure that returns name
> for any error that libusb may return, so we do not need
> to analyze error values by ourselves.
>
> Signed-off-by: Yuri Benditovich
Before applying the series:
(master 15e06ead) $ grepi "spice_usbutil_libusb_strerror" src/
src/win-usb-dev.c:116:const char *errstr =
spice_usbutil_libusb_strerror(rc);
src/win-usb-dev.c:173:const char *errstr =
spice_usbutil_libusb_strerror(rc);
src/channel-usbredir.c:312: spice_usbutil_libusb_strerror(rc), rc);
src/usbutil.c:62:const char *spice_usbutil_libusb_strerror(enum libusb_error
error_code)
src/usbutil.h:31:const char *spice_usbutil_libusb_strerror(enum libusb_error
error_code);
src/usb-device-manager.c:284:const char *desc =
spice_usbutil_libusb_strerror(rc);
src/usb-device-manager.c:311:const char *desc =
spice_usbutil_libusb_strerror(rc);
src/usb-device-manager.c:733:errstr =
spice_usbutil_libusb_strerror(errcode);
src/usb-device-manager.c:1071:const char *desc =
spice_usbutil_libusb_strerror(rc);
After applying the series:
(yuri-usb-b-layers-v1 5f87d90d) $ grepi "spice_usbutil_libusb_strerror" src/
(yuri-usb-b-layers-v1 5f87d90d) $
So, I think it makes sense to use this patch to drop this
function and always use libusb_error_name() instead, agree?
I checked that all above have include
I'll also be looking into your other patches but it might take
some time to get familiar with the changes :)
Cheers,
> ---
> src/usbutil.c | 32 +---
> 1 file changed, 1 insertion(+), 31 deletions(-)
>
> diff --git a/src/usbutil.c b/src/usbutil.c
> index e96ab11..4aa6ef7 100644
> --- a/src/usbutil.c
> +++ b/src/usbutil.c
> @@ -61,37 +61,7 @@ static usb_vendor_info *usbids_vendor_info = NULL;
> G_GNUC_INTERNAL
> const char *spice_usbutil_libusb_strerror(enum libusb_error error_code)
> {
> -switch (error_code) {
> -case LIBUSB_SUCCESS:
> -return "Success";
> -case LIBUSB_ERROR_IO:
> -return "Input/output error";
> -case LIBUSB_ERROR_INVALID_PARAM:
> -return "Invalid parameter";
> -case LIBUSB_ERROR_ACCESS:
> -return "Access denied (insufficient permissions)";
> -case LIBUSB_ERROR_NO_DEVICE:
> -return "No such device (it may have been disconnected)";
> -case LIBUSB_ERROR_NOT_FOUND:
> -return "Entity not found";
> -case LIBUSB_ERROR_BUSY:
> -return "Resource busy";
> -case LIBUSB_ERROR_TIMEOUT:
> -return "Operation timed out";
> -case LIBUSB_ERROR_OVERFLOW:
> -return "Overflow";
> -case LIBUSB_ERROR_PIPE:
> -return "Pipe error";
> -case LIBUSB_ERROR_INTERRUPTED:
> -return "System call interrupted (perhaps due to signal)";
> -case LIBUSB_ERROR_NO_MEM:
> -return "Insufficient memory";
> -case LIBUSB_ERROR_NOT_SUPPORTED:
> -return "Operation not supported or unimplemented on this platform";
> -case LIBUSB_ERROR_OTHER:
> -return "Other error";
> -}
> -return "Unknown error";
> +return libusb_error_name(error_code);
> }
>
> #ifdef __linux__
> --
> 2.17.1
>
> ___
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
signature.asc
Description: PGP signature
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 1/3] usb: use native libusb procedure for getting error name
libusb has libusb_error_name procedure that returns name
for any error that libusb may return, so we do not need
to analyze error values by ourselves.
Signed-off-by: Yuri Benditovich
---
src/usbutil.c | 32 +---
1 file changed, 1 insertion(+), 31 deletions(-)
diff --git a/src/usbutil.c b/src/usbutil.c
index e96ab11..4aa6ef7 100644
--- a/src/usbutil.c
+++ b/src/usbutil.c
@@ -61,37 +61,7 @@ static usb_vendor_info *usbids_vendor_info = NULL;
G_GNUC_INTERNAL
const char *spice_usbutil_libusb_strerror(enum libusb_error error_code)
{
-switch (error_code) {
-case LIBUSB_SUCCESS:
-return "Success";
-case LIBUSB_ERROR_IO:
-return "Input/output error";
-case LIBUSB_ERROR_INVALID_PARAM:
-return "Invalid parameter";
-case LIBUSB_ERROR_ACCESS:
-return "Access denied (insufficient permissions)";
-case LIBUSB_ERROR_NO_DEVICE:
-return "No such device (it may have been disconnected)";
-case LIBUSB_ERROR_NOT_FOUND:
-return "Entity not found";
-case LIBUSB_ERROR_BUSY:
-return "Resource busy";
-case LIBUSB_ERROR_TIMEOUT:
-return "Operation timed out";
-case LIBUSB_ERROR_OVERFLOW:
-return "Overflow";
-case LIBUSB_ERROR_PIPE:
-return "Pipe error";
-case LIBUSB_ERROR_INTERRUPTED:
-return "System call interrupted (perhaps due to signal)";
-case LIBUSB_ERROR_NO_MEM:
-return "Insufficient memory";
-case LIBUSB_ERROR_NOT_SUPPORTED:
-return "Operation not supported or unimplemented on this platform";
-case LIBUSB_ERROR_OTHER:
-return "Other error";
-}
-return "Unknown error";
+return libusb_error_name(error_code);
}
#ifdef __linux__
--
2.17.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
