read() returns greater than 1 with some C standard library implementations (i.e. newlib) about editline HOT 13 CLOSED

I'm maintaining my own patch for now, and I'm looking at possibly adding an AmigaOS driver that uses native AmigaDOS operations for handling the terminal (like with the existing UNIX and OS-9 driver code in the library). In the meantime someone who has been working on the Amiga newlib has said that the problem should be on their side, although I can't seem to get the problem to reappear outside of libeditline -- something with how the library interacts with termios seems to be what triggers the behaviour from what I've been able to determine.

from editline.

If this actually happens, it's most certainly not a standard library implementation because the standard does not permit that behavior. Do you have a reference to back up newlib actually doing this? It's very wrong and will break lots of legitimate code.

from editline.

I agree with Rich here, if we cannot trust the APIs to be POSIX compliant, then there's a lot more that could go very wrong.

Curious, how would you expect the editline project to handle this?

from editline.

I made the following change to editline.c:241 to get the library to work:

         r = read(el_infd, &c, 1);
     } while (r == -1 && errno == EINTR);
 
-    return r == 1 ? c : EOF;
+    return r >= 0 ? c : EOF;
 }
 
 static int tty_get(void)

I guess I should note that the environment is AmigaOS 4.1 using newlib as the C library. It could be that this is caused by a bug by Hyperion in their newlib port, and I'm looking at chasing that down as well.

from editline.

I don't see how you can meaningfully interpret a return value greater than nbytes here. What is it supposed to mean? That more bytes were written, resulting in a buffer overflow and arbitrary data having been clobbered? Or that 1 byte was written but the wrong value was returned for some reason? You simply can't know since this behavior is completely outside the contract of the read function. Whatever's going on, there's a serious bug in this newlib port and it needs to be tracked down to determine how to fix it. Papering over it in applications is not going to go well.

from editline.

I doubt that it'd be fixed in AmigaOS newlib any time soon, but my own testing on the platform has demonstrated to me that editline isn't losing any input at least.

from editline.

Again, a bug this serious is not something you can paper over. You need to find what the problem is and get it fixed. Otherwise there may be silent data clobbering/buffer overflow going on that you're not aware about but that's introducing mysterious bugs elsewhere. Or the behavior you're seeing might even be the result of UB elsewhere rather than a bug in the library (clobbering of the argument space, for instance, so that read thinks it got called with a larger nbytes).

If it is a bug in the newlib port, I don't see why you wouldn't expect an immediate fix you can apply and that would be available to others in right away and in the next release.

from editline.

If it's a bug in the newlib port, I'm stuck until Hyperion ships out a fix. Newlib licensing means that they're allowed to distribute their newlib.library without the sources used to build it. All I can do is file a bug with them with the test case I'm currently building, and hope they get to it. Considering their focus has changed to a different branch of AmigaOS lately, it's not likely that there will be an update for this, and so the only solution I can do is work around for this. In the meanwhile, my workaround works for AmigaOS; bytes after c appear to just contain junk and the next read() call returns the next character typed without anything skipped.

from editline.

You can read the binary and see what it's doing. And perhaps patch it.

from editline.

You might be able to do so, but currently that deep level of debugging is above my pay grade especially since PPC is not an ISA I'm used to.

from editline.

I'm sorry @coldacid, like @richfelker already said, this is not something that should be fixed in every library or application using read(). So I'm closing this as won't fix. You can always maintain a patch separately for your builds.

from editline.

@coldacid That definitely sounds more plausible, check rl_ttyset() for how to set your terminal in RAW mode. Would not be at all surprised if you only need to focus on that to fix your root cause.

from editline.

Per the developer working on termios in AmigaOS, it's completely a bug on their side, although it's already been fixed in an update of newlib.library that has yet to be released publicly. I guess the real solution is providing a custom rl_getc() depending on the newlib.library version, although that passes the buck onto the app developer.

from editline.