On Thu, Feb 06, 2014 at 01:25:50PM +0100, Damir Jelić wrote:
> On Thu, Feb 06, 2014 at 12:53:37AM +0100, Aurelien Jacobs wrote:
> > On Wed, Feb 05, 2014 at 02:29:12PM +0100, Damir Jelić wrote:
> > > On Tue, Feb 04, 2014 at 11:45:22PM +0100, Aurelien Jacobs wrote:
>
> > > > [...]
>
> > > > diff --git a/hardware/common/scpi_usbtmc.c
> > > > b/hardware/common/scpi_usbtmc.c
> > > > index 6cd7964..1ffc34a 100644
> > > > --- a/hardware/common/scpi_usbtmc.c
> > > > +++ b/hardware/common/scpi_usbtmc.c
> > > > @@ -37,6 +37,29 @@ struct usbtmc_scpi {
> > > > int response_bytes_read;
> > > > };
> > > >
> > > > +static GSList *scpi_usbtmc_scan(struct drv_context *drvc)
> > > > +{
> > > > + GSList *resources = NULL;
> > > > + GDir *dir;
> > > > + const char *dev_name;
> > > > + char *resource;
> > > > +
> > > > + (void)drvc;
> > > > +
> > > > + if (!(dir = g_dir_open("/sys/class/usbmisc/", 0, NULL)))
> > > > + if (!(dir = g_dir_open("/sys/class/usb/", 0, NULL)))
> > > > + return NULL;
> > > > + while ((dev_name = g_dir_read_name(dir))) {
> > > > + if (strncmp(dev_name, "usbtmc", 6))
> > > > + continue;
> > > > + resource = g_strconcat("/dev/", dev_name, NULL);
> > > > + resources = g_slist_append(resources, resource);
> > > > + }
> > > > + g_dir_close(dir);
> > > > +
> > > > + return resources;
> > > > +}
> > >
> > > This function is not portable,
> >
> > Really ? It only try to open two different directories, and if they do
> > not exist, it will just return cleanly. This should work on any OS that
> > do not have those 2 directories.
> >
>
> When I say it isn't portable I don't mean that it will break on other
> platforms, I mean that it won't do what it's supposed to do on other
> platforms. sr_usb_find_serial() wouldn't break either but it has a ifdef
> guard so it is clear that it isn't implemented for other platforms.
>
> > > you should guard it with an ifdef
> >
> > Actually this whole file won't do anything useful on non-linux OS. So
> > better not clutter it with ifdef and just exclude it from compilation on
> > anything non-linux.
> >
>
> I disagree, we should only guard the parts that need to be rewritten
> (under the assumption that we can do this without any breakage) so it's
> clear for other people what parts need to be implemented if they want
> this functionality on their platform.
The point is that this whole file is about supporting the linux only
/dev/usbtmc* device and kernel driver. AFAIK, no other OS has anything
looking like a usbtmc kernel driver exposed through a character device.
So in partice, this whole file is linux only and it is not possible
to port it on other platform.
That's why I think it is pointless (and harmful) to cripple this file
with some platform specific #ifdef.
And that's also one of the reason why I wrote a libusb based usbtmc
implementation: to have a portable usbtmc implementation.
Aurel
------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk
_______________________________________________
sigrok-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
