-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
ext/readline: readline_info fix usage when the buffer is not initialised #15139
Conversation
bea1cf3
to
3ab4ff7
Compare
Is this perhaps also related to this issue: ./configure --with-libedit
make
/php-src/ext/readline/readline.c: In function ‘zif_readline_info’:
/php-src/ext/readline/readline.c:191:41: warning: implicit declaration of function ‘rl_extend_line_buffer’ [-Wimplicit-function-declaration]
191 | rl_extend_line_buffer(Z_STRLEN_P(value) + 1);
| ^~~~~~~~~~~~~~~~~~~~~
/usr/bin/ld: ext/readline/readline.o: in function `zif_readline_info':
/php-src/ext/readline/readline.c:191:(.text+0x747): undefined reference to `rl_extend_line_buffer' The libedit is actually preferred library to use in PHP. See #13184 (readline library shouldn't even be an option to use in PHP). |
Not at all related but was not aware of the libedit/libreadline differences. |
ext/readline/readline.c
Outdated
if (!oldstr || strlen(oldstr) < Z_STRLEN_P(value)) { | ||
rl_extend_line_buffer(Z_STRLEN_P(value) + 1); | ||
} else if (!rl_line_buffer) { | ||
rl_line_buffer = malloc(Z_STRLEN_P(value) + 1); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused, surely having the if (!rl_line_buffer)
check first makes more sense? As you then don't need to check for !oldstr
?
3ab4ff7
to
3eb00aa
Compare
if the new value was larger, rl_line_buffer_length was never updated. neither was rl_end (on unixes). close GH-15120
Indeed, Licenses are NOT compatible, IMHO at some point we should drop libreadline support |
Let s try it for next major release then (php 9 ?). |
Quick check, If seems rl_end is not updated on Linux with libedit
But may be returned
|
rl_initialise is only called when readline() is used so the global buffer might not be initialised yet.
ff2b44f
to
56ec48a
Compare
Faced the same trying to build 8.4.0_alpha3 |
Thank you, the PR allowed to pass tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
It works, thanks! |
Will it cause the alpha3 tarball rebuild or better to wait for beta1? |
neither, there is alpha4. |
rl_initialise is only called when readline() is used so the global
buffer might not be initialised yet.