bug-readline
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-readline] [PATCH] Enable visibility annotations


From: Pedro Alves
Subject: Re: [Bug-readline] [PATCH] Enable visibility annotations
Date: Mon, 18 Apr 2016 12:18:18 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1

On 04/18/2016 11:57 AM, Yury Gribov wrote:
> On 04/18/2016 01:19 PM, Pedro Alves wrote:

>> I ran this now and got:
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Symbols which may be used and are missing in new libreadline:
>> rl_complete_with_tilde_expansion
>> _rl_mark_modified_lines
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> I wasn't expecting to ever see a "rl_" symbol here?  What was the
>> logic used to decide whether a symbol should be exported?
> 
> I only kept symbols which are present in public headers.

...

> All three are declared in a file with descriptive name rlprivate.h.

Ah.  I wonder why they're named that way instead of using
the _rl_ prefix?  In any case, seems to me that whatever policy applied
to _rl_ symbols used by applications should be applied to these,
and there's at least one program using rl_complete_with_tilde_expansion.

> 
>> There are other symbols here that look like should probably be public.
>> For example, history_offset is used by readline's examples:
>>
>>   readline/src/examples/hist_purgecmd.c:44:extern int history_offset;
>>   readline/src/examples/hist_purgecmd.c:142:       
>> history_offset--;     /* moving backwards in history list */
>>   readline/src/examples/hist_erasedups.c:41:extern int history_offset;
>>   readline/src/examples/hist_erasedups.c:114:          
>> history_offset--;         /* moving backwards in history list */
>>
>> so I'd assume that that's a public symbol and that those
>> examples no longer link after your patch?
> 
> Examples do link and run (most probably because they link static version
> of readline).

That may well be.  It'd be good to make sure the readline and history
examples still work with a shared library version of readline.

In this particular case, I have no idea whether it's the examples that
should be using something else or whether it's the symbol that should be
declared public.  But this looks like something that should be addressed
one way or the other.  Not good to be giving out examples that
don't work, IMO.

Thanks,
Pedro Alves




reply via email to

[Prev in Thread] Current Thread [Next in Thread]