Time Nick Message 00:21 rom1504 close them all 00:43 sofar the HUD api has me entirely baffled. I can't understand what any of the size/position parameters even do 00:44 sofar I just changed size to 0x0 and my hud size is the same :D 00:44 kaadmy yeah, it's really flaky 00:44 sofar there really needs to be like a gui editor that outputs the huddef for you 00:44 sofar lol 00:45 kaadmy for a while the ethic rush server had the HUD health/breath bars about 3 inches tall on the screen 00:45 kaadmy i don't think anybody else had that problem though 00:46 hmmmm the hud api can't really be fixed until a number of big things happen 00:46 hmmmm the first step is making a release 00:47 sofar I'm not even saying it's broken 00:47 sofar it's just ... mysterious 00:54 diemartin sofar, what hud_elem_type? 00:55 sofar I'm trying to make a text one with a image as background 00:55 diemartin pos and offset should work for most items 00:55 diemartin size and align is maybe broken for some 00:55 sofar I need scale for the bg img 00:55 diemartin scale rather 00:57 diemartin sofar, if scale > 0, use img_size*scale as size; if scale < 0, screen_size/100*(-scale) as size 00:57 sofar what's size? 00:57 diemartin is there a "size" thing? 00:57 sofar size = { x=100, y=100 }, 00:57 sofar -- ^ Size of element in pixels 00:57 sofar that's from lua_api.txt :D 00:58 diemartin that's new for me :P 00:58 sofar ohhh my text was way too large 00:58 sofar I have to \n my text to fit 06:51 sofar ssieb: fyi did you see digilines moved to minetest-mods? 06:51 ssieb yes 06:51 ssieb wasn't mesecons moving there too? 06:52 sofar jeija said "maybe later" 06:52 sofar I think he's close to that point, he just doesn't have that much time due to studies 06:54 sofar ssieb: considering the size, I'm contemplating making digilines a non-modpack 06:55 sofar I'm not sure that it needs to be split out. Could just keep the subdirs and directly include the code 06:55 sofar anyway, maybe later 06:56 ssieb The question is whether people would want only some of the items instead of all of them. 06:56 sofar right 06:57 ssieb It seems unlikely that someone would really *not* want some of the items. 06:57 sofar exactly 06:58 ssieb mesecons is similar though 06:58 ssieb but it's huge 07:03 sofar disabling the cmdblock is useful though 07:03 sofar but it could also be a separate mod, really 07:48 nore updated #3948 07:48 ShadowBot https://github.com/minetest/minetest/issues/3948 -- Escape more strings: formspecs, item descriptions, infotexts... by Ekdohibs 07:49 nore (there are two commits now, so it will be easier to review, I'll squash them before merging) 07:49 nore nrzkt: ^ 10:32 est31 btw, I didn't write csrp, I just ported it from an existing library 10:33 est31 and i did not do it to bypass code review or something 10:33 est31 in fact, it improved hugely thanks to hmmm's comments 10:33 est31 if anybody wants to improve it further, please feel welcome to PR 13:14 rubenwardy #3976 13:14 ShadowBot https://github.com/minetest/minetest/issues/3976 -- Add basic_priv property to privileges by rubenwardy 13:20 Obani Good idea 15:09 rubenwardy #3977 15:09 ShadowBot https://github.com/minetest/minetest/issues/3977 -- Add basic_privs setting by rubenwardy 15:55 Fixer !seen gregorycu 15:56 ShadowBot Fixer: I saw gregorycu in #minetest-dev 2 days, 1 hour, 7 minutes, and 42 seconds ago saying "If I am, I'd love a beer in the hot weather" 16:08 hmmmm oh my god 16:08 hmmmm can people stop making PRs or something 16:08 hmmmm it's not surprising at all we have over 130 pending because it keeps going back up to where it was every time some of them get merged 16:09 hmmmm too much stuff to track, too much stuff to review 16:13 rubenwardy lol 16:14 rubenwardy Minetest has a lot of minor things to fix 16:15 hmmmm well at least could somebody else hang around and prune PRs with me 16:15 rubenwardy Whenever I need to change Minetest's engine or builtin, I tend to make a PR 16:19 nore hmmmm: could you look at #3948 please? 16:19 ShadowBot https://github.com/minetest/minetest/issues/3948 -- Escape more strings: formspecs, item descriptions, infotexts... by Ekdohibs 16:20 nore IMHO, this should be merged in 0.4.14 as it is compatibility code for the future 16:20 hmmmm what does this do? 16:20 nore it finds escapes sequences in the strings that are displayed 16:20 nore and removes them 16:21 nore so that in the future, we can use such escape sequences for colored text or things like this 16:21 nore without causing strange effects on old clients 16:21 hmmmm christ... 16:21 hmmmm removeEscapes() looks horrifying 16:22 nore I've got a working but still needing work implementation of colored chat and item descriptions, etc too 16:22 nore should I try to make it simpler? 16:22 hmmmm no... it's just... 16:22 hmmmm i know it's not your code, but could you write a unit test for it? 16:23 hmmmm and \v == \x1B, I take it? 16:23 nore oh, of course :) (and actually, it is my code :D) 16:23 nore no, I changed it from the previous pr I made that was merged 16:23 nore that was merged a few weeks ago 16:24 nore because ShadowNinja asked for a more standard escape than \v 16:24 nore (it's in the commit message) 16:24 hmmmm okay so let me think about this 16:24 nore anyway, writing unittests then 16:25 hmmmm you have an escape character that people are meant to write in their textual strings 16:25 hmmmm but it's not the string literal \, x, 1, b, it's actually the character '\x1B' 16:25 hmmmm how do they write some text attribute? 16:26 nore what do you mean? 16:26 nore well, how it works is: 16:26 nore an escape is either "\x1b" 16:26 nore or "\x1b()" 16:27 hmmmm yes, i'm asking you how the end user writes the escape character 16:27 nore for example, the string "\x1b(fsfqqs)abcdef\x1beg" becomes "abcdefg" 16:28 nore ah, it is not possible for an user to type it in the chat for example 16:28 hmmmm what is the point of it exactly then..? 16:28 hmmmm mod only? 16:28 nore if we want to allow users to type colored text, there needs to be a mod that parses user messages and adds such escapes 16:28 nore yep 16:28 nore but it also allows colored item descriptions, etc 16:29 nore (if end users could type such messages in chat, it would be too easy to abuse) 16:29 hmmmm an end user could just modify the client 16:30 hmmmm and when client side modding comes around, god help ya 16:30 hmmmm i think that's a flimsy justification for not making it accessible to the end user 16:30 nore well, there could be some kind of filtering too 16:31 nore actually, the main problem is that we don't want to break existing strings 16:31 hmmmm so unescape_string applies the escape code effects to the string, I take it..? 16:31 nore and that means, we have to use a character that is not yet in existing strings... 16:31 nore unescape_string removes the '\' that were used to escape characters 16:32 nore for formspecs for example 16:32 hmmmm but doesn't removeEscapes() remove all of the escape sequences? 16:33 nore no, it only removes the \x1b escapes 16:33 hmmmm so now there are more escapes... 16:33 nore and then, unescape_string is called to remove the '\' characters 16:33 hmmmm what other escapes than the \x1Bs are there? 16:33 nore just '\\' for '\' in a formspec, and '\[' for '[', etc. 16:34 hmmmm is any of this documented anywhere? 16:34 nore unescape_string just removes '\' and adds the following character 16:34 nore the formspec escapes are 16:43 nore hmmmm: how do you run unittests btw? 16:43 hmmmm --run-unittest 16:43 hmmmm --run-unittests 16:44 nore ok, doing that :) 16:45 nore hm, I get floating point exception in pcgRandom :/ 16:47 kaadmy speaking of random 16:48 nore fixed it, sending that in another pr 16:48 kaadmy any idea why using lua's random (or math.random, whatever) function doesn't work for some people? 16:49 kaadmy pixture uses the builtin functions for villages, some people have no villages 16:49 kaadmy works for me, for others villages never spawn 16:50 nore wait, not fixed actually 16:52 nore hmmmm: added unittests and changed the name 16:56 hmmmm i noticed in a few places you removed unescape_string()s from labels 16:56 hmmmm what's up with this? 17:02 nore labels and tooltips do the unescape_string call now 17:02 nore but only after remove_escapes 17:03 nore and thus, they can be fed with strings that have escapes sequences in them 17:03 nore that means there won't be much to change when they will want to do things with those escape sequences 17:07 hmmmm oh, so the responsibility of handling escapes has been removed from parseTextList and friends 17:07 hmmmm but what about parseVertLabel for instance? 17:08 hmmmm or parseTabHeader 17:09 nore parseVertLabel still has it 17:09 nore because it modifies the string by adding '\n' between each character 17:09 hmmmm and tabs? 17:09 nore parseTabHeader has it because it gives the string directly to Irrlicht IIRC 17:10 hmmmm hmmmm okay 17:10 nore so there is no other level of indirection 17:10 hmmmm you should probably point all this out somewhere 17:10 nore hmmmm, where? 17:11 hmmmm like, "// Generally, responsibility of unescaping strings has been shifted from the FormSpec parse methods to the draw methods, with two exceptions: blah blah and here's why" 17:11 hmmmm a comment somewhere around the beginning of the parsing methods, maybe? 17:11 hmmmm or maybe in the .h file 17:12 nore ok, will do that 17:12 hmmmm thanks 17:16 nore done 17:27 nore hmmmm: nested escape sequences are not supported except if you escape the inner parentheses, should I add that? 17:28 hmmmm yes 17:28 hmmmm add all variations 17:28 hmmmm things supported AND not supported 17:37 celeron55 that escape stuff is crazy; i hope most of it can be unit tested 17:38 celeron55 otherwise it will have regressions in no time with 100% certainty 17:39 celeron55 or even worse, be broken to begin with 17:50 hmmmm tbh formspecs should be unit tested 17:51 hmmmm GUIFormSpecMenu should be split into two classes: FormSpecParser and FormSpecDrawer 17:51 hmmmm mock out the drawing part with a test implementation of the Drawer class that just verifies test assertions, not draw stuff to the screen 17:52 hmmmm i think formspec has been in a state of disrepair for a while because of an extreme lack of discipline in its implementation and maintenance 17:54 nore hmmmm: actually, I think formspec should be completely rewritten 17:54 nore well, the parsing part at least 17:55 nore we should use a better format, and write them with tables in Lua I'd say 17:56 nore but... this is a huge work to do :/ 17:57 sfan5 >and write them with tables in Lua I'd say 17:57 sfan5 heyy i already proposed that some time ago 17:57 sfan5 but it can't be done before 0.5 17:58 nore sfan5: why? 17:58 nore if we don't deprecate the old method 17:58 nore and we provide a conversion to the old format for older clients, it should work 17:59 sfan5 if you're rewriting formspecs you will only be held back by backwards compat 17:59 nore yeah, right 17:59 sfan5 some things should be rewritten without backwards compatibility 17:59 nore well, we should really start working to 0.5 sometime 17:59 sfan5 formspecs are one of them 17:59 sfan5 lel 17:59 sfan5 first we need to get .14 released 18:00 nore sapier's statbars could get into it too 18:00 nore yeah, but then we could start merging such changes 18:00 sfan5 only if we dont intend to have a 0.4.16 18:00 nore if we continue like now, we will merge those changes 18:00 sfan5 s/16/15/ 18:00 sfan5 or do you mean in a seperate branch? 18:01 nore well, if we intend to have a 0.4.15, it will get too complicated 18:01 nore we could make a dev-0.5 branch though, to merge *only* bugfixes in master 18:01 sfan5 there needs to be some point where we say: we'll start working on 0.5 now 18:01 nore and keep master a bit more stable 18:01 sfan5 after 0.4.14 is not that point imo 18:02 nore hm, when then? 18:02 nore the problem is that if we keep delaying, we will never start working on it :/ 18:02 hmmmm formspec is a total mess 18:02 hmmmm agreed 18:02 sfan5 maybe be more quicker with a following 0.4.15 release with more new features 18:02 sfan5 then start work on 0.5 18:02 hmmmm but it has way too much velocity to scrap 18:02 hmmmm err, i mean momentum 18:03 hmmmm in any case, if you abstract GUIFormSpecMenu in the manner i described, you become able to add new kinds of "parsers" and maintain the same "drawer" class 18:04 hmmmm my original plan was to have FormSpec as just one possible parser out of others 18:04 hmmmm I know people hate XML but it's really good for GUIs 18:04 sfan5 pls no 18:04 sfan5 are lua tables not good enough? 18:04 hmmmm and there are also other GUI markup languages out there 18:04 hmmmm lua tables would be another obvious frontend 18:04 hmmmm and they already exist in the form of sapier's FormSpec ToolKit 18:05 sfan5 i think i wrote some code that takes formspec table too sometime 18:05 sfan5 maybe i can find it somewhere.. 18:05 sfan5 (lua code) 18:05 hmmmm so I think what the lua table frontend would do is basically take the formspec toolkit and rewrite it as a real formspec frontend instead of a simple translation layer 18:05 hmmmm and another layer would be a simple drawing API 18:06 hmmmm we can reimplement the Hud API as a use of formspec 18:06 hmmmm there's lots of potential here but before work can start on that we need lots of other stuff to happen first 18:07 hmmmm and like I said the other day... a release would be a damn good place to start 18:07 sfan5 well we first need a new freeze date 18:07 hmmmm okay you can bugfix all day if you really wanted to 18:07 hmmmm but new features need to happen eventually 18:07 sfan5 found my code: https://gist.github.com/sfan5/9b366299a7e3a2103ffc70cdc36806b6 18:08 hmmmm i want to get the block send metadata optimization in before release 18:08 hmmmm and sofar's optimization as well 18:08 hmmmm these are huge and ought to be a part of the release just because of the impact these two things have on gameplay 18:08 sofar the nodeupdate one? 18:08 hmmmm performance 18:08 hmmmm yes 18:08 sofar yeah, that's a big one performance wise 18:09 hmmmm so there's those and a bunch of other misc. smaller PRs i'd like to get in just because i can't find a reason to not ship a release with them 18:09 hmmmm sfan, if you're going to be around later today I'd like to merge some things 18:09 hmmmm or anybody else 18:09 sfan5 i'll most likely be around 18:09 hmmmm also going to do a final over on nore's formspec escape thing 18:09 hmmmm yea 18:09 hmmmm more do less talk 18:09 hmmmm bbl 18:10 hmmmm there are a bunch of ShadowNinja PRs that i would have liked in before release but he's not very active... 18:10 hmmmm oh well 21:08 ssieb est31: what is csrp? 21:08 est31 ssieb, https://github.com/est31/csrp-gmp 21:08 est31 its a library I ported from openssl to libgmp 21:08 est31 used by minetest for authentication 21:09 ssieb thanks 21:09 est31 hmmmm, I couldn't notice any impact on the gameplay while I've tested the metas PR 21:12 est31 and about moving to 0.5 ... idk it would mean that we break compat, without a release in between that sustains compat. 21:12 sfan5 est31: https://github.com/est31/csrp-gmp/blob/master/srp.c#L545-L553 this is very dangerous, it should just fail if it can't get random bytes 21:12 est31 idk whether thats worth it 21:13 est31 hmmm 21:13 est31 good idea 21:14 est31 sfan5, for an "explanation" for how it got there: the error returning was only added by a more recent patch 21:14 est31 so before, you couldn't return an error 21:14 est31 see https://github.com/est31/csrp-gmp/commit/d97da9e7c72b4ab84a47173aa08acad9ab6aad93 21:15 est31 thats why I had to do the else part at all 21:15 sfan5 it should still be removed 21:15 est31 yes 21:15 est31 I'll do it 21:15 est31 one moment 21:21 est31 seems windows has similar issues 21:22 est31 (as in: not checking) 21:29 nore est31: btw, was it you who wrote PcgRandom? 21:30 est31 no 21:30 est31 it was hmmmm i think 21:30 nore its unittests crash with a floating point exception 21:30 nore hm... 21:30 est31 i only fixed a bug or so 21:30 nore ah, ok 21:30 nore I also get serialize/deserialize errors with floating point (precision probably) 21:33 nore est31: what's the problem with syntax coloring btw? 21:33 est31 default is a c++ keyword 21:34 est31 like new 21:34 est31 or class 21:34 nore ah 21:35 nore anyway, it's a separate commit IIRC 21:35 nore I can just drop it on squash 22:02 est31 ahh finding errors as I read through the code 22:02 est31 it really sucks to have no destructors 22:02 est31 would have worked greatly here 22:02 * est31 makes goto salad 22:03 est31 everybody, you must eat it, its healthy! 22:07 kaadmy why not find the cake? ;) 22:07 est31 :) 22:09 kaadmy or go portal hunting? 22:10 kaadmy for the last couple hours, trying to find a bug that is unreliable 22:10 kaadmy in a doom script :| 22:10 kaadmy either my problem(i have a few lines of code that are breaking, so chance is ~0%), or a mod i'm putting on top is breaking 22:14 est31 hahaaa 22:14 est31 typical error 22:14 est31 *bytes_v = (unsigned char*)srp_alloc(*len_v); 22:14 est31 if (!bytes_v) goto error_and_exit; 22:14 est31 spot the error :) 22:17 sofar allocating the length of a pointer? 22:17 est31 no , thats not the problem 22:18 est31 len_v is part of the return value 22:18 sofar sorry, that would have been the typical error ;) 22:18 est31 its not known by the calling code 22:18 est31 but derived 22:19 est31 the problem is, when i give a tip, its already too easy 22:19 est31 but think of it: does the if below make any sense? 22:19 MoNTE48 Hi guys. 22:20 est31 hello MoNTE48 22:20 MoNTE48 I wrote a little text. 22:20 MoNTE48 https://github.com/minetest/minetest/issues/3794#issuecomment-207861934 22:21 MoNTE48 While I was writing this, I thought that I should try again. I would like to move in Minetest some code that I have done for of MultiCraft, but not to do extra work I would like to discuss them with you first. 22:25 MoNTE48 Perhaps it would be cool if I did it to 0.4.14. I was planning to leave Minetest and MultiCraft soon, and I do not want my code and my ideas were gone. 22:31 est31 hmm, I don't want to take on that task, I am busy with many other things already, most of them not minetest. Perhaps somebody else? 22:32 est31 As an alternative, you can license your project under LGPL version 2.1. I know why you changed the license, but can you perhaps settle the conflict with nrz? 22:35 MoNTE48 I just want to make a small gift Minetest players, I do not want to solve the "problem" with Minetest devs. I have no problems, I took the LGPL code and do with it what I allowed a license. I do not have to ask permission from NRZ, celeron or other respected developers. 22:37 betterthanyou710 hi 22:38 celeron55 wtf was that 22:38 est31 ? 22:39 sofar I didn't think multicraft changed the license 22:39 est31 it did 22:39 est31 to lgpl 3 22:39 sofar are we looking at the same? 22:39 sofar https://github.com/MoNTE48/MultiCraft 22:39 celeron55 the readme doesn't say so 22:39 sofar are you thinking of proller's fork? 22:40 MoNTE48 https://github.com/MultiCraftProject/MultiCraft 22:40 celeron55 oh, well 22:40 sofar oh, that 22:40 celeron55 it's clear what changing a license to an incompatible one means though 22:40 sofar yeah, we can't :take: your code MoNTE48. you have to :submit: it 22:40 sofar emphasis on YOU 22:40 celeron55 you can't sugar coat it; you simply make the license incompatible and essentially say "fuck off" 22:41 sofar MoNTE48: nobody can take code from multicraft legally and put it in minetest, EXCEPT you 22:41 MoNTE48 I want to give it, but I can not change the license, or can someone block me without warning. So I do not want to create a lot of commits, that no one wants to check. 22:41 sofar if it's your code, you CAN change it 22:42 hmmmm I have no idea what multicraft is, or why it's so great that we'd want it for our project 22:42 hmmmm and then there's the license issue 22:43 sofar MoNTE48: the only way forward is you submitting PRs to minetest. anything else is a copyright or license violation. 22:43 MoNTE48 I chose GPLv3 only because some developers do not fully understand the essence of the LGPL, and Google moderators very stupid. I would like to discuss your changes and if they will be of interest to someone, I'll add them in Minetest. It will be an honor for me. 22:44 MoNTE48 Just play it. And then see Official Minetest on play store. https://play.google.com/store/apps/details?id=mobi.MultiCraft 22:45 est31 one thing is true: multicraft is far more successful than minetest 22:45 MoNTE48 (screenshots are very outdated, better download the game) 22:45 est31 (at least the app) 22:45 est31 factor 100 more installations 22:49 sofar MoNTE48: I assure you that if you submit PRs for actual bugs that they will get looked at. But nothing can be done if you don't submit patches.... 22:49 sofar unless you relicense, of course 22:49 celeron55 what even is the actual issue here 22:49 est31 relicensing would be easiest here, and it would be a nice signal 22:50 celeron55 i'm not sure if i understand MoNTE48's comment on github; does it mean that MoNTE48 wants to change minetest's android version to be the same as multicraft? 22:50 MoNTE48 est31, what do you mean? 22:50 sfan5 unrelated: the current HEAD builds fine on alpine linux (musl libc & busybox, so somewhat special) 22:50 hmmmm i'm willing to be the main reason why minetest lags behind multicraft in terms of downloads is because of the name :X 22:51 hmmmm sorry but minetest is so awkward sounding 22:51 Fixer ahah 22:51 Fixer minetest = "my test" i guess 22:51 hmmmm and that's just downloads, if it's free to download many people will 22:51 MoNTE48 I want to do management, MainMenu, Java part, default settings, build optimization and some minor corrections that I made for my game. We talk about the engine, not subgame. 22:51 Fixer we are testing the celerons code 22:53 celeron55 MoNTE48: i don't think anyone is willing to dig through multicraft's source and commits to see beforehand whether all of that would be accepted 22:53 celeron55 MoNTE48: to me it seems that most probably would, though 22:54 sofar well, I don't see anything wrong with people helping MoNTE48 going through it 22:54 celeron55 i mean if someone is willing to, then go ahead 22:55 MoNTE48 celeron55, I do not want someone digging around in my code. I just want to know whether you want this feature (which you can see in action by downloading my game) before I'll pull request. 22:55 celeron55 as far as i understand, the feature you refer to is actually quite many features 22:56 celeron55 even i can't make a blanket statement to allow it all right away before a PR exists; all i can say is most of it is probably ok 22:56 MoNTE48 celeron55, I can write the first thing that comes to my mind, and if you agree with this 2+ devs, I'll pull request. 22:58 celeron55 i guess we can say that the concept is approved, but it still has to pass code review 22:59 MoNTE48 This is what I wanted to hear. How much time do I have? It would be nice if it was added in 0.4.14. 23:00 hmmmm no features are going to be rushed into the next release 23:00 hmmmm however long it takes to review correctly is however long it'll take 23:00 MoNTE48 By the way. I've been wanting to ask. celeron 55, you would agree with that made NRZ? You have the right to cancel his application and unlock me. 23:00 hmmmm if it happens to miss the next release; oh well 23:01 est31 its not such a strict rule 23:01 celeron55 MoNTE48: i am not aware of enough details of that to say anything 23:02 MoNTE48 hmmmm, as I said before, I'm not a programmer. Absolutely. So I do not suggest to the "big features". Only minor bugfix, and improvements. 23:02 est31 but yes its good to not rush things 23:02 hmmmm really we had enough BS to deal with over things that were "urgent" 23:03 hmmmm in fact, these days, if somebody tells me that it's "urgent" to get a commit in, that makes me want to take a bit more time to scruitinize it even closer than i otherwise would have 23:03 celeron55 MoNTE48: if you are referring to a DMCA claim, then i would need to hear all details before i can tell if a right thing has happened 23:04 MoNTE48 celeron55, we can talk somewhere else, if you have the time? nothing can be changed now, I just want to satisfy my curiosity. 23:05 est31 https://github.com/est31/minetest/commit/4a53c72831d99dea16e82cc8ea10c819588f547b anybody wants to revie? 23:05 est31 review* 23:05 est31 it implements what sfan5 requested 23:05 est31 and much more :) 23:05 MoNTE48 I want to 0.4.14 was standard MainMenu with some corrections which I made, for example. It's not a lot of changes. Rather, nudalenie unnecessary functions. 23:05 celeron55 MoNTE48: it's 2 AM here now so i would prefer some other day and time for sure 23:06 hmmmm oh god 23:06 sfan5_ est31: referring to commits with only 5 chars looks weird to me 23:06 celeron55 anyway, if you want to discuss about a DMCA claim, then be prepared with everything you know about the case 23:06 MoNTE48 celeron55 2AM for me, too 23:06 hmmmm *I* reviewed csrp? 23:06 hmmmm how could i miss huge bugs like that 23:06 sfan5_ est31: check fread return value! 23:07 sfan5_ hm 23:08 sfan5_ is the numbering of enums guaranteed or can the compiler decide? 23:08 hmmmm and none of the parameters are checked for NULL before dereferencing 23:08 MoNTE48 I have a little extra time, now I'll MainMenu. And also do pull for my defaultsettings 23:08 hmmmm fine if it's your own code; but this is a publically facing library function 23:08 est31 it is guaranteed 23:08 MoNTE48 hmmm - for me? 23:09 est31 sfan5_, and the first case is always set to 0. 23:11 celeron55 what if we published an APK with some cooler name just to see if it would get more downloads? 8) 23:12 MoNTE48 celeron55 :P 23:15 MoNTE48 what is max_simultaneous_block_sends_per_client ? 23:16 est31 sfan5_, https://github.com/est31/minetest/commit/4d6279753038dd3011389202eaa86b4f4f0b6ffa 23:18 est31 aww 23:18 est31 there is an error with it 23:22 est31 hmm there is an error in checking the retvals 23:22 sfan5_ fread(a, b, 1, c) returns 1 on success 23:22 sfan5_ not b 23:22 sfan5_ http://man7.org/linux/man-pages/man3/fread.3.html 23:25 est31 no? 23:25 est31 ah 23:25 est31 yes 23:26 est31 thanks 23:27 sfan5_ also you don't need to check the fclose return value 23:27 sfan5_ there is no data to be flushed so it can't fail 23:28 sfan5_ it wouldn't impact functionality anyway 23:29 est31 well there are more reasons why fclose can fail 23:29 est31 " EINTR The close() call was interrupted by a signal; see signal(7)." 23:29 est31 EBADF fd isn't a valid open file descriptor. 23:29 sfan5_ EBADF is impossible 23:29 est31 even if there is no flush 23:29 est31 yeah 23:29 sfan5_ and EINTR is automatically retried by newer libc versions iirc 23:30 est31 well more checking is better than less checking 23:30 est31 updating the commit 23:30 sfan5_ checking fclose seems useless to me ¯\_(ツ)_/¯ 23:30 hmmmm sfan5 is correct 23:31 hmmmm checking the return of the close operation is useless - what are you going to do if it fails? 23:31 hmmmm tell the user that the operation failed, so they attempt to try it again perhaps? 23:31 est31 at least i know there was an error 23:31 est31 and then return it 23:31 celeron55 meh, english is a boring language because literally every good-sounding combination of words is someone's trademark already 23:31 celeron55 but communismcraft is still unused 23:31 hmmmm the user has no idea that the error was only on the close 23:31 celeron55 it was my april fool's prank back then, but i think it just might work 23:32 hmmmm the random bytes are still valid if fclose failed for whatever 23:32 paramat heheh 23:32 est31 celeron55, it has hype potential 23:32 sfan5_ same with CryptReleaseContext too 23:32 est31 no 23:32 sfan5_ why not 23:32 est31 CryptReleaseContext has even more cases where it can fail 23:32 est31 e.g. when the crypt resource is busy 23:32 sfan5_ yeah but the random bytes are still valid 23:33 est31 they are, but better throw an error 23:33 est31 one error better than no error 23:33 hmmmm i can't see any valid logical reason for that 23:33 sfan5_ it has no impact on functionality 23:33 hmmmm but it's your library, I guess, so you are exclusively responsible for bug reports 23:33 sfan5_ except a memory leak 23:33 est31 that's the kind of thinking that introduced Y2K 23:34 sfan5_ no 23:34 est31 hmmmm, the moment somebody finds a way for how to reproduce a failing close call, it becomes valid 23:35 sfan5_ oh also a word on code style 23:35 sfan5_ + if (!update_hash_n(alg, &ctx, A)) return SRP_ERR; 23:35 sfan5_ and 23:35 sfan5_ + if (!CryptGenRandom(wctx, sizeof(g_rand_buff), (BYTE*) g_rand_buff)) 23:35 sfan5_ + return SRP_ERR; 23:36 est31 yes, what's with it? 23:37 hmmmm it's a mishmash of styles 23:37 hmmmm we can't tell you what to do because it's your library, not a part of minetest, but sfan is just letting you know it's sloppy 23:37 est31 the first is on one line so that read flow doesnt break, its nicely lined up 23:37 est31 the second one is to not generate too long lines 23:37 sfan5_ that's not consistent 23:38 sfan5_ those reasons sounds like excuses 23:38 sfan5_ s/s // 23:38 est31 if (!CryptAcquireContext(&wctx, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) return SRP_ERR; 23:38 hmmmm guys we never came to a resolution the other night 23:38 est31 that's 101 characters 23:38 est31 longer than minetest's line limit 23:38 hmmmm est31: you don't seem to care, you have several lines waay over 100 characters 23:39 sfan5_ hmmmm: which productive activity do you propose 23:39 hmmmm I think we need to decide: 23:39 hmmmm - do we review third party libraries? 23:39 hmmmm - if so, to what level? 23:39 hmmmm - should libraries with their sources included be reviewed? 23:39 est31 aaaaaaaggggh you now got me to write a clang-format file just for csrp 23:40 hmmmm - what sort of pedigree do we require a third party library to have? 23:40 hmmmm - what sort of support do we expect for our libraries? 23:41 hmmmm I mean we clearly can't have Billy Bob's Awesome Framework included with 500 foritfy warnings and several security vulnerabilities 23:41 hmmmm but where is the line drawn 23:41 sfan5_ we don't need to review lua though (imo) 23:41 hmmmm right, and i agree. but why? 23:42 est31 Because we have irrlicht, and it is bad enough to have that buddy around? 23:42 * est31 hides 23:43 sfan5_ maybe the rule is to only review 'security critical' stuff 23:43 hmmmm https://github.com/est31/minetest/commit/4d6279753038dd3011389202eaa86b4f4f0b6ffa#diff-2f6369328116e9b654242a7597658a48R490 23:43 hmmmm ^ not concerned about line length here 23:43 sfan5_ one could argue that lua is security critical though 23:43 hmmmm idk 23:43 sfan5_ sqlite3 is clearly not 23:43 hmmmm everything is potentially security critical 23:44 hmmmm maybe not exactly remote code execution, but oftentimes a DoS 23:44 hmmmm if a server has a DoS vulernability I'd say that's pretty bad too 23:44 sfan5_ agreed 23:45 hmmmm in any case, you're not going to be able to review all of lua or sqlite 23:45 hmmmm you'll just take it for granted, use the library, and when there's a CVE for that library you then scramble to go update it (if it's included with your code) 23:46 hmmmm lua is a highly respected project with lots of activity and smart people working on it but it's not immune to bugs 23:46 hmmmm but i'd say there is a much lower chance of finding a security vulnerability in that than billy bob's authentication lib 23:47 hmmmm i just want to come up with some hard metrics we can use to determine if a library needs a closer look before including it 23:48 sfan5_ the srp library is essentially self-built/self-maintained 23:48 sfan5_ no other project uses it 23:48 hmmmm :\ 23:48 sfan5_ and it most likely won't have anyone looking for vulns in it 23:48 hmmmm could be a bad thing in the long run imho 23:48 sfan5_ (not this particular version at least) 23:48 hmmmm what license is csrp? 23:49 hmmmm MIT... hmmm 23:49 hmmmm I think the minetest project should "fork" csrp so the rest of us can develop/fix/improve it 23:49 hmmmm then at least we have a guaranteed level of support 23:49 hmmmm i.e. ourselves 23:50 sfan5_ are you sure there will be this much to fix? 23:50 hmmmm if we find something in the future, sure' 23:50 sfan5_ it seems more useful to just spend a few days sending est some patches 23:50 sfan5_ and then we'd be done with it 23:50 sfan5_ no need for a fork 23:51 hmmmm this is security *critical* code and only one person developed it, i reviewed it and apparently not too well because i somehow missed that potential null dereference that was fixed in the backport 23:52 sfan5_ https://github.com/est31/csrp-gmp/commit/8d33413276698d25008a614f37a6998952bf1f6c 23:52 sfan5_ why does this look so horrible 23:52 sfan5_ http://stackoverflow.com/questions/26433772/why-does-openssl-cleanse-look-so-complex-and-thread-unsafe hm