[00:00] Is there anyone here works in the IT department ? [00:00] however it is not entirely clear to me from the comments in that bug that the segfault is in fact within isprint [00:00] cjwatson: Cool, so I think that bug 965341 is that it should be using iswprint [00:00] Launchpad bug 965341 in procps (Ubuntu) "watch command line utility crashes with segfault when processing binary output" [Medium,Triaged] https://launchpad.net/bugs/965341 [00:00] cjwatson: comment #3 from me ? [00:00] I don't see how isprint could be getting inlined into main [00:01] that comment shows the segfault in main, afaics [00:01] yeh that's what the backtrace is showing - I'd thought the isprint and friends were macros or the like [00:01] i'm also curious how penguin42 inferred about isprint() from the actual bug. [00:02] they might be implemented as macros, true [00:02] oh, strace :-) [00:02] juliohm: he correlated gdb's output against the source code [00:02] oh, not strace, gdb output, nice. [00:03] yeah, isprint is a macro that expands to an array lookup [00:03] if the value is out of bounds then it might well segfault [00:03] cjwatson: Yeh, see ctype.h - it's a macro to __isprint that ends up indexing __ctype_b_loc that's 384 bytes long ish [00:03] Is there anyone here works in the IT department ? [00:03] Is there anyone here works in the IT department ? [00:03] not necessarily iswprint, it might also be correct to simply check that c is representable as unsigned char before passing it to isprint [00:04] Yick: please don't repeat without adding further context. Why do you want such a person? [00:04] (whose IT department?) [00:04] any IT department. Want to know anyone has connection to Microsoft. [00:04] cjwatson: However I'm a bit uncomfortable saying it just needs a change from isprint to iswprint since I don't understand the wide character stuff it's wrangling with; it does have a && c<128 *after* the !isprint(c) - but I think it needs a little more understanding [00:05] * penguin42 checks the date - oh yeh [00:05] penguin42: judging from the c<128 check, it isn't really interested in wide characters; I would say just !isprint(c) && c<128 => c<128 && !isprint(c) [00:06] * juliohm is curious about the details of following the execution of the watch command in gdb for backtracing the bug and fix it [00:06] juliohm: it prints a line number when the program crashes, so it's pretty easy to look up from there (in this case) [00:07] for the rest, man gdb :-) [00:07] cjwatson: Yeh, the code seems to be trying to detect wide characters and set a 'carry' variable to indicate it should fetch the next byte - and I think that's probably still OK [00:07] juliohm: I installed the procps debug sym package, I then ran gdb `which watch` and then ran it in gdb with your args [00:07] cjwatson, so, you have to download the source code first, right? [00:07] juliohm: yes, of course [00:07] juliohm: Then I grabbed the source (using apt-get source procps) and looked for line 541 [00:07] debugging programs without source code is no fun [00:08] hmm, very interesting. [00:08] let me try now, because maybe i can help in the future [00:09] penguin42, what do you mean by procps debug sym package? is just the procps package? [00:09] juliohm: Then I looked at that line and thought there's nothing that should seg fault, printed c, noticed it was HUGE, and wondered about isprint since I'd implement it using an array [00:09] hmm, mind you, isprint on a wint_t is a kind of dodgy thing to do [00:09] juliohm: https://wiki.ubuntu.com/DebuggingProgramCrash [00:10] * juliohm is reading the link... [00:10] cjwatson: Well, it does take an int (to allow EOF) [00:11] penguin42: sure, but technically, nothing in the standard requires int == wint_t [00:11] nod [00:11] penguin42: I think your first instinct was correct and it should be iswprint [00:11] assuming that tests out [00:12] cjwatson: What worries me is that at that point it would have the first half of a wide character and then passing that to iswprint also makes no sense [00:12] penguin42: how so? my_getwc appears to read a whole wide character [00:13] or WEOF [00:14] penguin42: in any case, I think the important thing is that iswprint does *not* have a range of values specified as undefined behaviour [00:15] oh no that's not true [00:15] "For all functions described in this subclause that accept an argument of type wint_t, the [00:15] value shall be representable as a wchar_t or shall equal the value of the macro WEOF. If [00:15] this argument has any other value, the behavior is undefined. [00:15] " [00:15] still, my_getwc always returns something that meets that description, I think [00:16] given that it always returns either rval (which is a wchar_t and gets implicitly converted) or WEOF [00:16] cjwatson: I'd be more comfortable if I understood what the heck that loop was actually trying to do [00:17] I haven't bothered to work it out but I'm confident in my statement that iswprint should never segfault in this case from dataflow analysis alone :) [00:18] it's some tedious display mangling, doesn't seem terribly interesting to work out the exact details [00:18] yeh, I'm just not sure it does what the oriignal code was supposed to [00:20] well, the while condition basically asks for a non-printing character under 128 that isn't newline, tab, or (sometimes) esc; so promoting to iswprint for type compatibility doesn't seem to break anything there [00:20] and my_getwc blows my mind - I can't see how it's while loop ever executes more than once given that it always returns in one of two ways [00:20] everything that manipulates c is typesafe [00:21] oh, the if isn't indented - ok, I see how my_getwc works [00:21] penguin42: it just has rotten indentation [00:21] yeah, the last return is conditional on 'byte == MAX_ENC_BYTES' [00:22] but again, I think you just need to verify that it always returns a promoted wchar_t or WEOF [00:22] which is pretty clear [00:23] it's fairly grotty code but just stick to analysing the bits you need and it's ok [00:24] yeh, and so my_getwc either a valid WC, a WEOF on an EOF< or WEOF on a bad char, then I think I agree iswprint should work [00:24] the one thing I'd note is the FORCE_8BIT bit at the top of the file, which may or may not be obsolete now [00:24] although leaving it there wouldn't actually hurt anything [00:26] yeuch [00:26] penguin42, thank you very much for the link, it's really helpful. ;-) [00:26] I'll try the process when i have time. [00:27] cjwatson: The code there comes from one of the debian patches [00:27] cjwatson: watch_unicode.patch [00:27] yep [00:28] is that a big deal? [00:28] I was just wondering if it was a debian patch or an ubuntu patch, but I think it's a debian one [00:29] looks like it [00:29] nod [00:30] cjwatson: Is the right thing there to add another patch to the end of the set or to fix that patch? [00:31] judgement call in general - in this case I'd probably fix that patch [00:31] ode for a 1 character tweek [00:31] though I wouldn't reject a branch that added another one, I guess [00:31] but yeah, seems overkill [00:32] actually it needs one of the other patches tweeking as well because that is diff'd against it [00:33] quilt push; quilt refresh your way up the stack [00:33] but basically trivial [00:35] I could look at that tomorrow, but it's rather late now, and I can't actually remember any of teh quilt foo [00:37] if you've tested a patch and confirmed it works, feel free to just attach it to the bug and I can take care of the packaging [00:37] but you might want the learning experience and if so I don't want to deprive you either :) [00:37] I'll look tomorrow, or should I say at 1:37 am later today [00:43] right, bed! === vorian is now known as v === v is now known as Obama === Obama is now known as vorian === vorian is now known as heHATEme === Guest89637 is now known as jalcine === Guest16852 is now known as vibhav [06:52] https://launchpad.net/ubuntu/precise/+queue :D [06:54] YokoZar: Yeah, I'm trying to decide if that was a joke. :P [06:54] YokoZar: Do I review it or reject it? [06:54] infinity: Review it of course, it fulfills its stated purpose ;) [06:54] Oh dear... [06:55] Oh, derp. April 1. [06:58] * micahg is curious if a second person signed off on it [07:04] cjwatson: see that you have been updating openssl. could you add the elliptic curve compile flag for 64bit for the next round? thx. [08:59] jhojho: no, I haven't got round to talking with the Debian maintainer about it yet [09:23] cjwatson: Hi, could you perhaps sponsor another package? -> mail-notification, bug 881039 [09:23] Launchpad bug 881039 in mail-notification (Ubuntu) "mail-notification-evolution only contains files in /usr/share/doc" [Undecided,Confirmed] https://launchpad.net/bugs/881039 [09:24] Whoopie: not right now, need to go out [09:25] cjwatson: ok [09:25] have fun [09:25] I can look later if nobody beats me to it [09:30] Whoopie: you might find that some sponsors are unwilling to upload, without a Firstname Lastname in debian/changelog. [09:31] Daviey: might be. Even though I see a lot of packages with the same "issue". === tkamppeter_ is now known as tkamppeter [10:06] Whoopie: It's not terribly common for people to upload with a real name. [10:06] Whoopie: But if the code's good, I don't care deeply either. === juliank__ is now known as juliank === Pici is now known as ZarroBoogs [11:18] ZarroBoogs: awww <3 === jalcine_ is now known as Guest91350 === Guest91350 is now known as jalcine === dendro-afk is now known as dendrobates === dendrobates is now known as dendro-afk === bladernr_ is now known as bladernr_afk [15:24] Hi all! === dendro-afk is now known as dendrobates === mnepton is now known as mneptok === jbicha is now known as Guest20909 === Guest20909 is now known as jbicha_ [16:20] tjaalton: hey, i thought i'd seen a push of xserver-xorg-video-qxl go by, but I don't see it in the archive? [16:21] (maybe I mis-read a subject and it was another xserver-xorg...) === dantti is now known as dantti_away === dendrobates is now known as dendro-afk === dendro-afk is now known as dendrobates [16:35] hallyn: you think we can just sync it? I mean it probably needs an FFe bug just to be on the safe side :) [16:36] tjaalton: I don't know, since Munzir hasn't responded I guess I will try installing kubuntu in a guest tomorrow and see if I can (a) reproduce the bug and (b) see it fixed with the new version. [16:37] tjaalton: if it doesn't explicitly fix a bug, then you might be right we should file ffe. [16:37] guess it coudln't hurt to file ffe anyway, and then comment there if it fixes a bug [16:38] (really, spice should be seen as a tech preview right now) [16:40] hallyn: I don't see any bugs open against -qxl, so this was something reported on irc? [16:40] tjaalton: bug 913314 [16:40] Launchpad bug 913314 in xserver-xorg-video-qxl (Ubuntu) "Corrupted display with Spice" [High,Fix released] https://launchpad.net/bugs/913314 [16:41] after confirmation of the fix of original bug, he lists a few other things still going wrong. [16:41] ah, ok [16:44] * penguin42 looks up === dendrobates is now known as dendro-afk === dendro-afk is now known as dendrobates === dendrobates is now known as dendro-afk === dendro-afk is now known as dendrobates === yofel_ is now known as yofel [18:05] Is it possible for me to obtain a correct source of TAI from my system if I keep it in sync? [18:06] Implementing TAI on top of UTC relies on UTC being correctly implemented. But Ubuntu seems to ignore leap seconds in it's implementation of UTC. === dendrobates is now known as dendro-afk [19:19] hello === jalcine is now known as jalcine_ === heHATEme is now known as vorian === mwhudson_ is now known as mwhudson === nijaba_ is now known as nijaba