--- Log opened Mon Nov 18 00:00:53 2024
00:57 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
01:07 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
03:32 -!- VonNaturAustreVe [~natur@177.96.233.162] has joined #secp256k1
03:32 -!- VonNaturAustreVe [~natur@user/vonnaturaustreve] has changed host
03:55 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
03:59 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
04:32 -!- ne0h_ [~natur@189.6.18.93] has joined #secp256k1
04:35 -!- VonNaturAustreVe [~natur@user/vonnaturaustreve] has quit [Ping timeout: 260 seconds]
04:48 -!- ne0h_ [~natur@189.6.18.93] has quit [Quit: Leaving]
04:52 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
05:15 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
05:57 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
06:07 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
06:38 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
07:01 < sipa> meeting?
07:01 < nickler> hi
07:01 < sipa> nickler: nearby?
07:01 < hebasto> hi
07:02 < nickler> sipa: potentially, I'm in midtown manhattan. Not sure where you're located.
07:02 < real_or_random> hi
07:03 < real_or_random> topics? :)
07:03 < real_or_random> josie suggested "better type hints for fixed size arrays" on nov 14th
07:04 < nickler> no particular topic from me
07:06 < sipa> let me ping him
07:07 < real_or_random> quoting his message
07:07 < real_or_random> 15:11 <josie> brief context: we often pass pointers to arrays of unsigned char, where the length of the array is fixed
07:07 < real_or_random> 15:11 <josie> it was suggested in https://github.com/bitcoin-core/secp256k1/pull/1479#discussion_r1741694956 , that we could do this in such a way to provided better hints to language binding tooling, specifically rust
07:07 < real_or_random> 15:11 <josie> considering this for the silent payments module , but felt it would be good to discuss here
07:11 < real_or_random> taking into account Kixunil's last message in the mentioned issue, this would  probably boil down to declaring arguments like `unsigned char arg[32]` instead of `unsigned char *arg32`
07:12 < sipa> or const unsigned char (*arg32)[32]?
07:12 < real_or_random> yeah but you pointed out that this is not backwards compatible?
07:12 < real_or_random> what is this actually supposed to mean? a pointer to an array of size 32?
07:12 < sipa> ABI compatible (probably), but not API-compatible, so it'd only be usable for new APIs (or if we decide the break)
07:13 < sipa> yeah, it's a pointer to a 32-byte array
07:13 < real_or_random> but no double-indirection then? if you say it's ABI-compatible
07:13 < sipa> which in (reasonable) ABI terms is really a pointer to its first element
07:13 < sipa> right, you can't implicitly cast a pointer to an array to a pointer to its first element
07:14 < real_or_random> C even requires this, so this should really hold true in any ABI
07:14 < sipa> which is the point, you shouldn't be able to do that!
07:14 < sipa> but the fact that it's a break is annoying
07:15 < sipa> but for a new module, it would be an option
07:15 < sipa> except that inconsistency is somewhat annoying
07:15 < real_or_random> hm, yeah, I'd anyway prefer arg[32] because it's more readable
07:15 < sipa> yeah, but it doesn't mean anything at all
07:16 < sipa> while misleading people to think it does
07:16 < real_or_random> it is self-documenting and tools like bindgen can make use of it
07:17 < sipa> i'm not a fan
07:17 < real_or_random> I see
07:17 < sipa> unless we actually introduce static analysis tools that are able to detect misuse
07:17 < real_or_random> what does (*arg)[32]  "mean"?
07:18 < sipa> say you have void foo(unsigned char (*arg)[32]) { ... }
07:18 < sipa> then this works: unsigned char bla[32]; foo(&bla);
07:19 < sipa> and inside foo, you'll need to access say the last byte of that array as (*arg)[31]
07:20 < real_or_random> but isn't this a "double-indirection" then? the argument is a pointer to a pointer to a char then?
07:21 < sipa> as far as the C language is concerned, yes
07:21 < real_or_random> but why is it not an ABI break then?
07:22 < sipa> but it gets compiled to just passing the address of the first byte of bla
07:23 < real_or_random> hm, okay, I don't see why
07:23 < sipa> not just compiled
07:23 < sipa> (void*)arg == (void*)(&((*arg)[0])) will be true
07:24 < sipa> so the actual pointer address stored in arg will be identical to the pointer to the first byte of the array pointed to
07:24 < josie> sorry guys, got the time mixed up for this meeting
07:27 < sipa> (arrays in C are weird)
07:27 < real_or_random> ha, I still don't get it... arg[0] is defined as *(arg+0) == *arg. so &((*arg)[0]) == &**arg == *arg != arg
07:27 < josie> sipa: "but for a new module, it would be an option" in the context of the silent payments module, my main question was is this something worth doing for the module, given that its expected to be used by languages like rust
07:27 < real_or_random> but it's probably me, sorry ^^
07:28 < sipa> real_or_random: (*arg)[32] means pointer to an array of 32 chars
07:28 < sipa> in that setting, arg[0] is "array of 32 chars", which is a pointer to the first of those 32 chars
07:29 < sipa> arg[1] is the *next* array of 32 chars, which is a pointer to the first of the 32 chars of the second elements, i.e., a pointer 32 bytes further than arg[0]
07:29 < sipa> &arg[0][0] would be the first byte of the array of 32 bytes, which is the same address as arg[0] itself, as is normal for C arrays
07:30 < sipa> &arg[0][31] would be a pointer to the last byte of the first 32-byte array
07:30 < sipa> &arg[1][0] would be a pointer to the first byte of the second 32-byte array, so 1 byte further than &arg[0][31]
07:33 < sipa> 10:27:28 < real_or_random> ha, I still don't get it... arg[0] is defined as *(arg+0) == *arg. so &((*arg)[0]) == &**arg == *arg != arg
07:34 < sipa> this is literally describing how C arrays work
07:34 < sipa> say you have "char c[32];" somewhere
07:35 < real_or_random> ok that makes sense
07:35 < sipa> then (&c == &(c[0])) will be true
07:36 < real_or_random> ok yep
07:36 < sipa> they have distinct types, but after collapsing to void*, they are equal
07:36 < real_or_random> josie: hm, yeah, nickler said in the PR "I prefer to have a standard across the whole API and would avoid just changing this in the musig module."  
07:36 < real_or_random> looking at the assembly here also convinces me https://godbolt.org/z/z5o8eooq8 .. and now I understand, thanks
07:36 < real_or_random> (yeah, it's weird) 
07:37 < sipa> i believe the technical term is actually "bat-shit insane"
07:37 < real_or_random> :x
07:37 < real_or_random> I'm obviously biased here but I'm not a fan of that API then because I think it's a bit uncommon and hard to understand
07:38 < sipa> yeah, agreed there
07:38 < real_or_random> roconnor brought up "arg[static 32]", which is somewhere in the middle
07:38 < real_or_random> but that's super uncommon, and C99
07:39 < sipa> yeah, so it'd need ifdefs
07:39 < sipa> but it's more readable, and actually has fairly sane semantics
07:40 < real_or_random> yeah or some kind  of "SECP256K1_STATIC" within the square brackets  , which is ... at least ugly
07:40 < real_or_random> but it seems that compilers try to warn if you try to pass a too-small array or NULL 
07:40 < real_or_random> (which is very neat)
07:41 < sipa> indeed
07:41 < sipa> and it has another advantage of (*arg)[32]
07:41 < sipa> int you have "unsigned char data[32]", then foo(&data)" will not work with (*arg)[32]
07:42 < sipa> where foo(data) will work with "arg[static 32]"
07:43 < real_or_random> perhaps that's a solid reason to require C99 
07:43 < real_or_random> it's 25 years in the end
07:44 < real_or_random> the old gcc version on godbolt is 3.4.6, released March 06, 2006.... It eats the "static" without any issues
07:45 < sipa> hmm
07:45 < sipa> interesting
07:45 < sipa> is it API & ABI compatible?
07:45 < sipa> i would assume so
07:46 < real_or_random> oldest clang too
07:46 < real_or_random> but the *most recent* MSVC fails... face-against-wall
07:46 < sipa> wahaha
07:46 < real_or_random> perhaps it needs the right compiler switch lol
07:46 < sipa> SECP256K1_STATIC here we come?
07:48 < josie> SECP256K1_STATIC would still have the same concern re API inconsistency? or is that since this appears to be backwards compatible, we could update all of the modules to use this convention?
07:49 < real_or_random> you could define it in an #ifdef ...
07:49 < sipa> josie: my belief is that "char *arg", "char arg[32]", and "char arg[static 32]" (where supported) are all compatible
07:49 < sipa> if so, there would not be any issues introducing that everywhere
07:49 < josie> sipa: nice! 
07:49 < sipa> but it's not C89
07:49 < sipa> so it may mean some ifdeffing
07:50 < real_or_random> yeah, I don't think we can convince MSVC to understand this "static"
07:51 < real_or_random> probably related to their lack of VLA support
07:51 < sipa> yeah
07:52 < sipa> #define SECP256K1_ARRAY(typ, len, nam) typ[static len] nam
07:52 < sipa> or
07:53 < sipa> #define SECP256K1_ARRAY(typ, len, nam) typ* nam
07:53 < real_or_random> this could work
07:53 < real_or_random> hm... not elegant, but it would do the job
07:54 < sipa> we'd need to be absolutely sure this is both ABI and API compatible (except of course the cases which we actually want to outlaw, like passing pointer to too-short array)
07:57 < real_or_random> I'm rather confident for this syntax 
07:57 < real_or_random> it's just an additional promise to the compiler
07:58 < sipa> right
08:00 < real_or_random> what we already have is this NOT_NULL thing. but since this is a function attribute, it doesn't hamper the legibility of the declaring too much
08:01 < sipa> right
08:01 < sipa> msvc doesn't seem to define __STDC_VERSION__, which is necessarily set for C99
08:01 < sipa> so it's detectable too
08:02 < real_or_random> right
08:02 < sipa> real_or_random: does MSVC accept the static thing when "/std C11" or "/std C17" is passed?
08:02 < josie> (gotta run, but thanks for the discussion on this! i learned a lot)
08:02 < real_or_random> okay, I think I wouldn't be opposed, but I'm also not convinced that something like this is a net gain in the end 
08:03 < sipa> also, does rust bindgen deal with this correctly?
08:03 < real_or_random> no, I tried this.. which is strange. they claim they support all *required* features of C11, but apparently no (or it's optional?)
08:03 < real_or_random> https://stackoverflow.com/questions/78584708/c99s-non-null-array-syntax-pointers-for-parameters-for-msvc also no meaningful answer here
08:04 < real_or_random> ah this is a good question, let's just ask  Kixunil  in this conversation
08:05 < real_or_random> I posted a comment
08:05 < sipa> the "type[static len]" syntax is part of the VLA feature, which is required in C99 but optional in later standards
08:05 < real_or_random> ah I see
08:05 < real_or_random> how did you figure this out?
08:05 < sipa> https://en.cppreference.com/w/c/language/array#Variable-length_arrays
08:05 < sipa> it's listed under VLA on cppreference :p
08:05 < sipa> which i assume is correct
08:06 < sipa> oh, nevermind
08:06 < real_or_random> ok
08:07 < real_or_random> well whatever, the reality is that MSVC doesn't support it
08:07 < sipa> indeed
08:07 < real_or_random> I have to run now, too. not sure what the conclusion is
08:08 < sipa> i'm open to trying to SECP256K1_ARRAY thing
08:08 < sipa> which we could use everywhere if our assumptions are correct
08:10 < real_or_random> makes sense
08:11 < real_or_random> if anything, it makes forces people to read the API in more depth, which is not too bad :D
08:12 < sipa> right
08:12 < sipa> we can use __STDC_NO_VLA__ to gate support for it, i guess
08:13 < sipa> and enable the static thing if (C99 || ((C11 or later) and not __STDC_NO_VLA__))
08:18 < hebasto> skimming through https://learn.microsoft.com/en-us/cpp/c-language/c-language-reference -- there seems no guarantees to comply with C99 standard
08:20 < sipa> hebasto: yes, that is well-known (certain features that are mandatory in C99 were made optional again later, specifically because MSVC refuses to implement them)
08:20 < sipa> but i do think they claim to support C11 and C17?
08:21 < sipa> so the question is how they get away with not supporting array[static]... and it must be that that's considered part of such an optional feature (or that's how MSVC chooses to interpret it...)
09:06 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
10:15 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
10:29 < roconnor> Are you sure the static syntax is VLA?  The cpp.reference link discusses it prior to the VLA section. 
10:29 < sipa> no, it's not; i was wrong
10:29 < sipa> (that was what my "oh, nevermind" was about, i wasn't very clear)
10:30 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
10:32 < sipa> but in practice, i'm guessing that "no-vla C11/C17" perfectly predicts "no support for array[static]"
10:33 < sipa> alternatively, it could be "(c99 or later) and not msvc"
10:36 < roconnor> Oh I should have read the whole log first. 
12:02 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
12:16 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
15:25 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
23:11 -!- theStack [~theStack@95.179.145.232] has quit [Ping timeout: 272 seconds]
23:59 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
--- Log closed Tue Nov 19 00:00:54 2024