[Rxtx] NRJavaSerial, a fork of rxtx that fixes the papercuts!

[Chris Bartley]

Exposing your class member collection to direct modification by user code should almost certainly be considered a bug.  As it stands now, there's nothing preventing someone from doing this:

	final RXTXCommDriver rxtxCommDriver = ...; // I haven't looked at the code carefully to see how one gets a reference...doesn't matter
	final Set<String> ports = rxtxCommDriver.getPortIdentifiers();
	ports.clear();

Imagine the case where your app has two threads, where each thread connects to a different serial device.  Maybe you had two different developers write the connection details for the different threads.  Developer A has the above code and then does this:

	ports.add("/dev/myspecialport");

...because maybe he knows the port name will always be the same and doesn't want to bother with any others.

But the Developer B doesn't know this and is surprised to find that, upon repeated testing, the Set returned by rxtxCommDriver.getPortIdentifiers() contains either 1) the expected Set of ports, 2) an empty Set, or a Set containing only "/dev/myspecialport".  All three scenarios are possible with the code as it stands now since the ports Set in RXTXCommDriver is static, both threads are working against the same Set, and there's no guarantee as to when Developer B's code which iterates over the set will run...either before Developer A's ports.clear(), after ports.clear() but before the ports.add(), or after the ports.add().

There's also an issue with two threads simultaneously calling rxtxCommDriver.getPortIdentifiers().  That's a potentially serious problem, but not immediately relevant to what we're discussing here.

Chris

On Mar 28, 2011, at 2:57 PM, Kevin Harrington NR wrote:

> >Either way, the code as it stands now is buggy because the client could modify your Set.
> >Chris
> >[1] http://code.google.com/p/nrjavaserial/source/browse/trunk/nrjavaserial/src/main/java/gnu/io/RXTXCommDriver.java
> 
> Well the Set<> was left modifiable by intention. If a user wanted to artificially add an option to the list (maybe to add in linux the /dev/serial/by-id/* dev nodes which are not mapped normally) then they are free to do that in user land for their application without having to create a new object. Likewise, if they added the /dev/serial/by-id/ node for a port, they would need to remove the duplicate that exists in the Set. I see no reasonable reason to remove this option from the user, mostly because having the option adds to the usability and does not detract from it. I would say that maybe the documentation has a bug by not stating that possible use case, but think it is unfair to label the code as buggy.  As i stated before, the goal is to make the library more friendly to the end user. 
> _______________________________________________
> Rxtx mailing list
> Rxtx at qbang.org
> http://mailman.qbang.org/mailman/listinfo/rxtx