RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 1393750 - Add fix for brltty to compile with OCaml 4.04.0
Summary: Add fix for brltty to compile with OCaml 4.04.0
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: brltty
Version: 7.4
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: rc
: ---
Assignee: Jaroslav Škarvada
QA Contact: Tereza Cerna
URL:
Whiteboard:
Depends On:
Blocks: 1384533 1434818
TreeView+ depends on / blocked
 
Reported: 2016-11-10 09:18 UTC by Jaroslav Škarvada
Modified: 2018-01-22 13:34 UTC (History)
10 users (show)

Fixed In Version: brltty-4.5-14.el7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of: 1392147
: 1434818 (view as bug list)
Environment:
Last Closed: 2017-08-01 23:03:13 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2017:2179 0 normal SHIPPED_LIVE brltty bug fix update 2017-08-01 19:40:46 UTC

Description Jaroslav Škarvada 2016-11-10 09:18:41 UTC
+++ This bug was initially created as a clone of Bug #1392147 +++

Description of problem:

See the failed build here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16300469

The build log is also attached.

Version-Release number of selected component (if applicable):

brltty-5.4-3.fc26

How reproducible:

Once.

Steps to Reproduce:
1. Build brltty in f26-ocaml side tag.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 11:43 CET ---

The attached patch fixed the problem for me, but maybe it's not correct (I don't know much about OCAML).

From the OCAML documentation (e.g. [1]):
> 19.5.1  Simple interface
> ...
> Rule 1
> ...
> In particular, CAMLlocal and CAMLxparam can only be called after CAMLparam.

In e.g. raise_brlapi_error and raise_brlapi_exception the CAMLlocal1 and CAMLlocal2 were used without CAMLparam.

The proposed patch seems to at least fix the compilation problem.

[1] http://caml.inria.fr/pub/docs/manual-ocaml-4.04/intfc.html

--- Additional comment from Richard W.M. Jones on 2016-11-08 12:02:06 CET ---

The patch is nearly but not quite right.

You mustn't pass non-"value" type arguments to CAMLparamX.  If
you try that then the garbage collector may crash when it tries
to follow those non-value pointers and interpret them as OCaml
heap objects.  So the second instance must use CAMLparam0() as well.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 12:04:59 CET ---

(In reply to Richard W.M. Jones from comment #2)
> The patch is nearly but not quite right.
> 
> You mustn't pass non-"value" type arguments to CAMLparamX.  If
> you try that then the garbage collector may crash when it tries
> to follow those non-value pointers and interpret them as OCaml
> heap objects.  So the second instance must use CAMLparam0() as well.

Thanks for info, I doubt the patch was correct :)

--- Additional comment from Jaroslav Škarvada on 2016-11-08 12:05 CET ---



--- Additional comment from Jaroslav Škarvada on 2016-11-08 12:06:47 CET ---

Jon, do you want to handle this bugzilla? I can do it myself as co-maintainer.

--- Additional comment from Richard W.M. Jones on 2016-11-08 12:11:44 CET ---

Patch looks good to me.  I'll add it and do the build as I
am coordinating everything in the f26-ocaml side tag.

--- Additional comment from Richard W.M. Jones on 2016-11-08 12:24:45 CET ---

Fix included in:
http://koji.fedoraproject.org/koji/taskinfo?taskID=16351380
brltty-5.4-4.fc26

Please send the patch upstream.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 13:18:02 CET ---

(In reply to Richard W.M. Jones from comment #7)
> Fix included in:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=16351380
> brltty-5.4-4.fc26
> 
> Please send the patch upstream.

Reassigning to myself, I will manage it with upstream.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 13:58:39 CET ---

Patch sent upstream.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 17:34:04 CET ---

Richard, upstream encountered the following warnings with OCaml-4.00 and the patch applied:

> ./brlapi_stubs.c: In function ‘raise_brlapi_error’:
> ./brlapi_stubs.c:147:29: warning: unused variable ‘caml__frame’
> [-Wunused-variable]
> ./brlapi_stubs.c: In function ‘raise_brlapi_exception’:
> ./brlapi_stubs.c:161:29: warning: unused variable ‘caml__frame’
> [-Wunused-variable]

This doesn't occur with OCaml-4.04. Is it bug? Is there a way how to workaround it?

--- Additional comment from Richard W.M. Jones on 2016-11-08 17:39:38 CET ---

It does occur with OCaml 4.04.0, it is a bug, and it is fixed by your patch
which I have now included in the package.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 17:41:57 CET ---

(In reply to Richard W.M. Jones from comment #11)
> It does occur with OCaml 4.04.0, it is a bug, and it is fixed by your patch
> which I have now included in the package.

Sure, I wonder whether there is a way how to workaround the warnings with older OCaml. Brltty upstream is curious. He needs to support multiple versions of OCaml in brltty.

--- Additional comment from Richard W.M. Jones on 2016-11-08 18:04:51 CET ---

Interesting ...  I believe this was NOT an error in older versions
of OCaml.  I'm not sure it's an error even in OCaml 4.04.

There has been a change to the CAMLxparam<N> macros in the header,
but at the moment I'm unclear why it causes this new warning.

Perhaps it's newer GCC being cleverer about finding unused variables.

--- Additional comment from Jaroslav Škarvada on 2016-11-08 18:26:08 CET ---

FYI upstream fix:
https://github.com/brltty/brltty/pull/81

It seems upstream is happy, so it's resolved for me. Thanks Richard.

--- Additional comment from Richard W.M. Jones on 2016-11-08 19:06:43 CET ---

Thanks for liaising with upstream on this issue.

--- Additional comment from Jaroslav Škarvada on 2016-11-09 11:52:28 CET ---

I used upstream patch in rawhide (just commited, not built) as according to upstream using CAMLparam0() without CAMLreturn0() can lead to runtime problems.

Comment 1 Jaroslav Škarvada 2016-11-10 09:20:19 UTC
There is an effort to rebase OCaml to version 4.03+ in RHEL-7 which is tracked in bug 1384533. This brltty fix is required for the rebase.

Comment 5 Richard W.M. Jones 2017-03-21 13:17:00 UTC
Adding the fix is fine, but as you're still building against
ocaml-4.01.0-22.7.el7_2.x86_64 this doesn't fix the bug.

In fact since IBM didn't file the required bugs to rebase OCaml,
we're not going to be fixing any of this in RHEL 7.4.

Can you also drop the advisory (from the menu on the top right
drop down button).

Comment 6 Richard W.M. Jones 2017-03-21 13:17:42 UTC
(In reply to Richard W.M. Jones from comment #5)
> Can you also drop the advisory (from the menu on the top right
> drop down button).

Well unless you've fix any other bugs, in which case don't do that!

Comment 7 Jaroslav Škarvada 2017-03-22 09:57:00 UTC
(In reply to Richard W.M. Jones from comment #5)
> Adding the fix is fine, but as you're still building against
> ocaml-4.01.0-22.7.el7_2.x86_64 this doesn't fix the bug.
> 
> In fact since IBM didn't file the required bugs to rebase OCaml,
> we're not going to be fixing any of this in RHEL 7.4.
> 
> Can you also drop the advisory (from the menu on the top right
> drop down button).

Sorry, why you dropped flags on the bug which has been already approved by devels, PM and QA? The subj is:
"Add fix for brltty to compile with OCaml 4.04.0" which the patch exactly do. The subj is not "recompile with OCaml 4.04.0". Moreover the fix is correct, approved by upstream and should be already there, because the code was wrong.

Comment 8 Richard W.M. Jones 2017-03-22 10:38:02 UTC
(In reply to Jaroslav Škarvada from comment #7)
> Sorry, why you dropped flags on the bug which has been already approved by
> devels, PM and QA? The subj is:
> "Add fix for brltty to compile with OCaml 4.04.0" which the patch exactly
> do. The subj is not "recompile with OCaml 4.04.0". Moreover the fix is
> correct, approved by upstream and should be already there, because the code
> was wrong.

Because to fix this bug you do actually need to rebuild brltty
against OCaml 4.04, whatever the bug description might say.

I'm not against including the fix.

Comment 9 Jaroslav Škarvada 2017-03-22 10:54:38 UTC
(In reply to Richard W.M. Jones from comment #8)
> (In reply to Jaroslav Škarvada from comment #7)
> > Sorry, why you dropped flags on the bug which has been already approved by
> > devels, PM and QA? The subj is:
> > "Add fix for brltty to compile with OCaml 4.04.0" which the patch exactly
> > do. The subj is not "recompile with OCaml 4.04.0". Moreover the fix is
> > correct, approved by upstream and should be already there, because the code
> > was wrong.
> 
> Because to fix this bug you do actually need to rebuild brltty
> against OCaml 4.04, whatever the bug description might say.
> 
> I'm not against including the fix.

No, the patch fixes the code which was wrong and as a bonus allows local recompilation with newer ocaml.

And btw you shouldn't drop flags on components which are not yours, it's impolite at least.

Comment 17 Jaroslav Škarvada 2017-05-16 13:13:25 UTC
Created attachment 1279329 [details]
OCaml simple test

For QA how to test that the OCaml bindings works:

I suppose you do not have Braille terminal, so the following steps are for the emulator (but in case you have Braille terminal it's just a matter of different driver option):

# yum install brlapi brlapi-devel brltty-xw ocaml-brlapi
$ ocamlc -o test -I +brlapi unix.cma brlapi.cma ./test.ml
$ brltty -b xw -x no -A auth=none,host=127.0.0.1:0
$ ./test

You should see the "Hello world!" on Braille display in both ASCII and Braille for 10 seconds.

Q&A:
Is it possible to automate it?

Of course, it is! Just use the Virtual driver and replace the last two steps with:
$ brltty -b vr -d server:127.0.0.1 -x no -A auth=none,host=127.0.0.1:0
$ telnet 127.0.0.1 35752
cells 20

Now in another terminal run:
$ ./test

In the first terminal you should see the text.

Conclusion? Be happy!

Comment 18 Richard W.M. Jones 2017-05-16 14:14:26 UTC
Here is my improved version of the program:

--------
open Brlapi
open Unix
open Printf

let () =
  try
    openConnection settings_initializer;
    enterTtyMode tty_default "";
    writeText cursor_off "Hello world!";
    sleep 10;
    leaveTtyMode ();
    closeConnection ()
  with
  | Brlapi_error err ->
    eprintf "Brlapi_error: %s\n" (strerror err)
  | exn ->
    prerr_endline (Printexc.to_string exn)
--------

However I get an error:

$ brltty -b xw -x no -A auth=none,host=127.0.0.1:0
$ ocamlc -g -o test -I +brlapi unix.cma brlapi.cma ./test.ml
$ ./test 
Brlapi_error: Can't determine tty number

--------

strace is not very informative.  The last part is ...

socket(PF_LOCAL, SOCK_STREAM, 0)        = 3
connect(3, {sa_family=AF_LOCAL, sun_path="/var/lib/BrlAPI/0"}, 110) = -1 ENOENT (No such file or directory)
futex(0x7ffa6093e620, FUTEX_WAKE_PRIVATE, 2147483647) = 0
close(3)                                = 0
socket(PF_INET, SOCK_STREAM, IPPROTO_TCP) = 3
connect(3, {sa_family=AF_INET, sin_port=htons(4101), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
setsockopt(3, SOL_TCP, TCP_NODELAY, [1], 4) = 0
read(3, "\0\0\0\4\0\0\0v", 8)           = 8
read(3, "\0\0\0\10", 4)                 = 4
sendto(3, "\0\0\0\4\0\0\0v", 8, 0, NULL, 0) = 8
sendto(3, "\0\0\0\10", 4, 0, NULL, 0)   = 4
read(3, "\0\0\0\4\0\0\0a", 8)           = 8
read(3, "\0\0\0N", 4)                   = 4
open("/proc/self/stat", O_RDONLY)       = 4
fstat(4, {st_mode=S_IFREG|0444, st_size=0, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7ffa61f08000
read(4, "5070 (test) R 5067 5067 4868 348"..., 1024) = 301
close(4)                                = 0
munmap(0x7ffa61f08000, 4096)            = 0
write(2, "Brlapi_error: Can't determine tt"..., 41Brlapi_error: Can't determine tty number
) = 41
exit_group(0)                           = ?
+++ exited with 0 +++

Comment 19 Jaroslav Škarvada 2017-05-16 20:42:23 UTC
(In reply to Richard W.M. Jones from comment #18)
> Here is my improved version of the program:
> 
> --------
> open Brlapi
> open Unix
> open Printf
> 
> let () =
>   try
>     openConnection settings_initializer;
>     enterTtyMode tty_default "";
>     writeText cursor_off "Hello world!";
>     sleep 10;
>     leaveTtyMode ();
>     closeConnection ()
>   with
>   | Brlapi_error err ->
>     eprintf "Brlapi_error: %s\n" (strerror err)
>   | exn ->
>     prerr_endline (Printexc.to_string exn)
> --------
> 
> However I get an error:
> 
> $ brltty -b xw -x no -A auth=none,host=127.0.0.1:0
> $ ocamlc -g -o test -I +brlapi unix.cma brlapi.cma ./test.ml
> $ ./test 
> Brlapi_error: Can't determine tty number
> 

Thanks, your enhanced version also works for me. I got two warnings, but functionality is OK:
File "./test2.ml", line 7, characters 4-39:
Warning 10: this expression should have type unit.
File "./test2.ml", line 8, characters 4-31:
Warning 10: this expression should have type unit.

Could you try explicitly setting the tty number instead of using the tty_default? I will retest on cleanly provisioned machine (so far I have tested it on my "stable" system and it worked OK) and I will try to reproduce the problem.

For QA also feel free to extend the test code. I wrote this very simple test, because there were none code on the Internet (or I didn't search so hard :).

Comment 20 Richard W.M. Jones 2017-05-17 13:57:40 UTC
Yes it does work once I changed:

-    enterTtyMode tty_default "";
+    enterTtyMode 1 "";

So it all looks good to me.

For future reference I was testing:

  ocaml-brlapi-0.6.0-16.el7.x86_64

which is my own-built version of brltty
(see https://bugzilla.redhat.com/show_bug.cgi?id=1434818#c2)

Comment 21 Jaroslav Škarvada 2017-05-17 14:38:44 UTC
It seems the default TTY doesn't work if I ssh to the machine. In such case the TTY number needs to be set explicitly. I am currently unsure whether it's bug or not. I will take a look.

Comment 22 Jaroslav Škarvada 2017-05-17 16:10:55 UTC
(In reply to Jaroslav Škarvada from comment #21)
> It seems the default TTY doesn't work if I ssh to the machine. In such case
> the TTY number needs to be set explicitly. I am currently unsure whether
> it's bug or not. I will take a look.

The auto-detection routine which is enabled with the tty_default detects the controlling TTY of the calling process. With the SSH connection there is no TTY, only the PTY, so the auto-detection routine doesn't detect anything, which is probably the right behaviour (we are not running from the TTY, so there is nothing to detect). In such case you need to specify the TTY number by hand. The TTY is not important for this simple test, so arbitrary TTY can be used. The only minor problem I can see is that the brlapi documentation could be more specific about the behaviour when the auto detection fails.

Comment 23 Tereza Cerna 2017-06-01 13:52:41 UTC
Tested in:
    brltty-4.5-15.el7

* patch applied
* it builds with ocaml-4.04 

-> VERIFIED

Comment 24 errata-xmlrpc 2017-08-01 23:03:13 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2017:2179


Note You need to log in before you can comment on or make changes to this bug.