Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return value of fcntl not checked #88

Closed
tibbe opened this issue Jun 1, 2016 · 5 comments
Closed

Return value of fcntl not checked #88

tibbe opened this issue Jun 1, 2016 · 5 comments

Comments

@tibbe
Copy link

tibbe commented Jun 1, 2016

At

int flags = fcntl(m_FD, F_GETFL, 0);
there's a call to fcntl which return value isn't checked:

int flags = fcntl(m_FD, F_GETFL, 0);

Since this call can fail we might pass an invalid value for flags just after:

flags &= ~O_NONBLOCK;
checkReturnCode(fcntl(m_FD, F_SETFL, flags));

This in turn can lead to an exception being raised, but at the wrong point, suggesting that what failed was the flag set, not the flag get!

@tibbe
Copy link
Author

tibbe commented Jun 1, 2016

I ran into this using the Celestron Firmware Manager, which uses this library. Here's the full stack trace:

09:29:36.268 _003.924.860 _ java.util.concurrent.ExecutionException: purejavacomm.PureJavaIllegalStateException: JTermios call returned -1 at class purejavacomm.PureJavaSerialPort line 1093
        at java.util.concurrent.FutureTask.report(FutureTask.java:122)
        at java.util.concurrent.FutureTask.get(FutureTask.java:192)
        at cfm.devicetask.DeviceTask.get(DeviceTask.java:390)
        at cfm.devicetask.DeviceTask.done(DeviceTask.java:300)
        at cfm.devicetask.DeviceTask.access$000(DeviceTask.java:94)
        at cfm.devicetask.DeviceTask$2.done(DeviceTask.java:134)
        at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:384)
        at java.util.concurrent.FutureTask.setException(FutureTask.java:251)
        at java.util.concurrent.FutureTask.run(FutureTask.java:271)
        at cfm.devicetask.DeviceTask.run(DeviceTask.java:380)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:745)
Caused by: purejavacomm.PureJavaIllegalStateException: JTermios call returned -1 at class purejavacomm.PureJavaSerialPort line 1093
        at purejavacomm.PureJavaSerialPort.checkReturnCode(PureJavaSerialPort.java:1294)
        at purejavacomm.PureJavaSerialPort.<init>(PureJavaSerialPort.java:1093)
        at purejavacomm.CommPortIdentifier.open(CommPortIdentifier.java:148)
        at cfm.link.SerialLink.acquireLinkInner(SerialLink.java:160)
        at cfm.link.SerialLink.acquireLink(SerialLink.java:126)
        at cfm.devicetask.SeekPrimaryTask.testPortal(SeekPrimaryTask.java:132)
        at cfm.devicetask.SeekPrimaryTask.doInBackground(SeekPrimaryTask.java:81)
        at cfm.devicetask.SeekPrimaryTask.doInBackground(SeekPrimaryTask.java:26)
        at cfm.devicetask.DeviceTask$1.call(DeviceTask.java:119)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        ... 4 more

It's hard to tell what the issue is as the raise on F_SETFL could have actually been triggered by the fcntl 2 lines above.

@nyholku
Copy link
Owner

nyholku commented Jun 1, 2016

Hi, thanks for the feedback. On cursory look I do not see that F_GETFL can fail and if it fails how can we check that it failed.

http://linux.die.net/man/2/fcntl

@tibbe
Copy link
Author

tibbe commented Jun 1, 2016

It can fail with EBADF in case of bad file descriptor (http://www.gnu.org/software/libc/manual/html_node/Getting-File-Status-Flags.html). In that case it will return -1 and errno will be set.

@nyholku
Copy link
Owner

nyholku commented Jun 1, 2016

Ok, the man page then leaves something to desire for ;) ... ok I will fix this, thanks for spotting and contributing.

@nyholku
Copy link
Owner

nyholku commented Oct 1, 2016

Ok, this has been addressed will be released in 1.0.1

@nyholku nyholku closed this as completed Oct 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants