search

jnr / jnr-posix

Java Posix layer
Other
241 stars 91 forks source link

handle UnsatisfiedLinkError in LinuxPOSIX constructor, resolves #187 #188

Closed betalb closed 9 months ago

betalb commented 9 months ago

POC PR that will resolve #187

enebo commented 9 months ago

@betalb So I have a question which you may know. The original code is written around each of these three foreign calls potentially being independently defined (lazily setting bool for each one to fall back when it is wrong). I would think typically it would be all or nothing (like your PR does) but it isn't how the rest of this code is written. The original statVersion test is an all or nothing field but the check only checks one stat.

If we could assume alll 3 calls are always defined or not defined we could get rid of all this try/catch stuff in the individual stat methods.

I think your PR looks ok to me and it definitely fixes an obvious problem on systems missing these calls.

@headius do you have any thoughts on this? I half think we should assume all or nothing based on presence of xstat64 and then remove the volatile booleans and the extra logic in each stat.

enebo commented 9 months ago

I should add this appears to be written around lazily working but it clearly only worked lazily ONLY is __xstat64 worked. So at a minimum we could eliminate that one from having volatile field and lazy logic.

enebo commented 9 months ago

@betalb could you change this to be a single final boolean for all 3? Since this happens in the constructor and we try once we can then remove all UnsatisfiedLinkErrors in the 3 stat methods and only check that boolean value. If you don't want to we can do it but it might be fun to delete code? :smile:

betalb commented 9 months ago

I did the changes, discussed here Also I mentioned that -1 return code handling was inconsistent between fstat and lstat/stat in versions that accept FileStat object as argument, so I've aligned them.

I can make another change and move error reporting to method versions with 2 arguments (int fd, FileStat stat). Though I see that super-classes follow the same pattern and report error in stat versions that accept only integer argument and return stat object.

headius commented 9 months ago

@betalb There's definitely an unfortunate lack of consistency in these APIs, as we have been slowly adding and improving them over the past 10-15 years. We would definitely welcome any additional PRs that make things more consistent!

I will review the updated PR.