2018-04-19 15:45 GMT+03:00 Bram Moolenaar <[email protected]>:
>
> Christ van Willegen wrote:
>
>> On Thu, Apr 19, 2018 at 12:12 PM, Bram Moolenaar <[email protected]> wrote:
>> >
>> > Nikolay Pavlov wrote:
>> >
>> >> 2018-04-19 0:01 GMT+03:00 Bram Moolenaar <[email protected]>:
>> >> >
>> >> > Patch 8.0.1735 (after 8.0.1723 and 8.0.1730)
>> >> > Problem: Flexible array member feature not supported by HP-UX. (John
>> >> > Marriott)
>> >> > Solution: Do not use the flexible array member feature of C99.
>> >> > Files: src/configure.ac, src/auto/configure, src/structs.h,
>> >> > src/getchar.c, runtime/doc/develop.txt
>> >
>> > [...]
>> >
>> >> > --- 511,517 ----
>> >> > struct buffblock
>> >> > {
>> >> > buffblock_T *b_next; /* pointer to next buffblock */
>> >> > ! char_u b_str[1]; /* contents (actually longer) */
>> >>
>> >> Why not just stuff a macros here which will expand to 1 or nothing,
>> >> depending on configure check for flexible array members support?
>> >> _FORTIFY_SOURCE point still stands and macros name is a nice
>> >> replacement for “actually longer” comment.
>> >
>> > Well, that means maintaining two versions of the code. And since many
>> > compilers do support flexible array members, the version still using [1]
>> > would get little testing, and it might break for some rare compilers in
>> > mysterious ways. I rather have one solution, even when it's not all
>> > that nice. In other words: Adding a nicer solution doesn't help getting
>> > rid of the not-so-nice solution.
>>
>> What Nikolay says is (maybe you interpreted it the same way, but just
>> to make sure..):
>>
>> - Add code to 'make config' that spits out a "#define
>> HAS_FLEXIBLE_ARRAY 1" (or "#define HAS_FLEXIBLE_ARRAY 0") into
>> auto/config.h, define a macro (in vim.h, probably) that does:
>>
>> #if HAS_FLEXIBLE_ARRAY
>> #define FLEX_ARRAY
>> #else
>> #define FLEX_ARRAY 1
>> #endif
>>
>> and implement struct buffblock (in structs.h as):
>>
>> /*
>> * structure used to store one block of the stuff/redo/recording buffers
>> */
>> struct buffblock
>> {
>> buffblock_T *b_next; /* pointer to next buffblock */
>> char_u b_str[FLEX_ARRAY]; /* contents */
>> };
>>
>> This way, you only need to write the code once, and the compiler gets
>> tested in both cases.
>>
>> If the compiler(s) all get flexible array compatibility, the need to
>> use this macro may disappear, but that has happened before...
>>
>> Is this a "nice enough" solution to get rid of the "not so nice" one?
>
> Please look at the change, it makes clear that just changing [1] to []
> does not work. With [1] you can include the struct in another struct,
> with [] the size is undefined and it won't work. It's possible to use
> the solution I made for [] in both cases, but this has several risks.
> E.g. not computing the size of the allocated memory correctly.
Based on what I know [] variant should work always. If size of the
allocated array is being computed with offsetof() + len (actually the
only way to do that correctly) the only thing you actually risk is
that it will allocate one byte less then sizeof() which is not a
problem as long as you neither access that byte (doing so would be a
bug with [] variant in any case) nor try to pass structure by value
(outright impossible with [] variant).
I do not see anything in this patch confirming I am wrong here.
You may “loose” a way to store structure with flexible array member
somewhere directly and not by pointer, but such a thing is easily
resolved by using “compatible structures” - structures with the same
members, except that flexible array member receives an actual size -
and casting them around when a pointer to flexible structure is
needed. And I believe this solution is nicer then things like
struct
{
dictitem_T var;
char_u room[VAR_SHORT_LEN];
} fixvar[FIXVAR_CNT];
(in Neovim I changed that to
TV_DICTITEM_STRUCT(VAR_SHORT_LEN + 1) fixvar[FIXVAR_CNT];
with
#define TV_DICTITEM_STRUCT(...) \
struct { \
typval_T di_tv; /* Structure that holds scope dictionary itself. */ \
uint8_t di_flags; /* Flags. */ \
char_u di_key[__VA_ARGS__]; /* Key value. */ \
}
: here __VA_ARGS__ basically lets choose between flexible array member
and fixed size, though implementation has different quirks: we do not
support compilers without C99 support, but we do support not too
outdated versions of MSVC which does not accept MACROS() as a valid
macros expansion if macros has one argument).
>
> --
> hundred-and-one symptoms of being an internet addict:
> 223. You set up a web-cam as your home's security system.
>
> /// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net \\\
> /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
> \\\ an exciting new programming language -- http://www.Zimbu.org ///
> \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
>
> --
> --
> You received this message from the "vim_dev" maillist.
> Do not top-post! Type your reply below the text you are replying to.
> For more information, visit http://www.vim.org/maillist.php
>
> ---
> You received this message because you are subscribed to the Google Groups
> "vim_dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
