[00:00] <Yick> Is there anyone here works in the IT department ?
[00:00] <cjwatson> however it is not entirely clear to me from the comments in that bug that the segfault is in fact within isprint
[00:00] <penguin42> cjwatson: Cool, so I think that bug 965341 is that it should be using iswprint
[00:00] <penguin42> cjwatson: comment #3 from me ?
[00:00] <cjwatson> I don't see how isprint could be getting inlined into main
[00:01] <cjwatson> that comment shows the segfault in main, afaics
[00:01] <penguin42> yeh that's what the backtrace is showing - I'd thought the isprint and friends were macros or the like
[00:01] <juliohm> i'm also curious how penguin42 inferred about isprint() from the actual bug.
[00:02] <cjwatson> they might be implemented as macros, true
[00:02] <juliohm> oh, strace :-)
[00:02] <cjwatson> juliohm: he correlated gdb's output against the source code
[00:02] <juliohm> oh, not strace, gdb output, nice.
[00:03] <cjwatson> yeah, isprint is a macro that expands to an array lookup
[00:03] <cjwatson> if the value is out of bounds then it might well segfault
[00:03] <penguin42> 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] <Yick> Is there anyone here works in the IT department ?
[00:03] <Yick> Is there anyone here works in the IT department ?
[00:03] <cjwatson> 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] <cjwatson> Yick: please don't repeat without adding further context.  Why do you want such a person?
[00:04] <cjwatson> (whose IT department?)
[00:04] <Yick> any IT department. Want to know anyone has connection to Microsoft.
[00:04] <penguin42> 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] <cjwatson> 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] <cjwatson> 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] <cjwatson> for the rest, man gdb :-)
[00:07] <penguin42> 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] <penguin42> 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] <juliohm> cjwatson, so, you have to download the source code first, right?
[00:07] <cjwatson> juliohm: yes, of course
[00:07] <penguin42> juliohm: Then I grabbed the source (using apt-get source procps) and looked for line 541
[00:07] <cjwatson> debugging programs without source code is no fun
[00:08] <juliohm> hmm, very interesting.
[00:08] <juliohm> let me try now, because maybe i can help in the future
[00:09] <juliohm> penguin42, what do you mean by procps debug sym package? is just the procps package?
[00:09] <penguin42> 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] <cjwatson> hmm, mind you, isprint on a wint_t is a kind of dodgy thing to do
[00:09] <penguin42> juliohm: https://wiki.ubuntu.com/DebuggingProgramCrash
[00:10]  * juliohm is reading the link...
[00:10] <penguin42> cjwatson: Well, it does take an int (to allow EOF)
[00:11] <cjwatson> penguin42: sure, but technically, nothing in the standard requires int == wint_t
[00:11] <penguin42> nod
[00:11] <cjwatson> penguin42: I think your first instinct was correct and it should be iswprint
[00:11] <cjwatson> assuming that tests out
[00:12] <penguin42> 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] <cjwatson> penguin42: how so?  my_getwc appears to read a whole wide character
[00:13] <cjwatson> or WEOF
[00:14] <cjwatson> 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] <cjwatson> oh no that's not true
[00:15] <cjwatson> "For all functions described in this subclause that accept an argument of type wint_t, the
[00:15] <cjwatson> value shall be representable as a wchar_t or shall equal the value of the macro WEOF. If
[00:15] <cjwatson> this argument has any other value, the behavior is undefined.
[00:15] <cjwatson> "
[00:15] <cjwatson> still, my_getwc always returns something that meets that description, I think
[00:16] <cjwatson> given that it always returns either rval (which is a wchar_t and gets implicitly converted) or WEOF
[00:16] <penguin42> cjwatson: I'd be more comfortable if I understood what the heck that loop was actually trying to do
[00:17] <cjwatson> 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] <cjwatson> it's some tedious display mangling, doesn't seem terribly interesting to work out the exact details
[00:18] <penguin42> yeh, I'm just not sure it does what the oriignal code was supposed to
[00:20] <cjwatson> 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] <penguin42> 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] <cjwatson> everything that manipulates c is typesafe
[00:21] <penguin42> oh, the if isn't indented - ok, I see how my_getwc works
[00:21] <cjwatson> penguin42: it just has rotten indentation
[00:21] <cjwatson> yeah, the last return is conditional on 'byte == MAX_ENC_BYTES'
[00:22] <cjwatson> but again, I think you just need to verify that it always returns a promoted wchar_t or WEOF
[00:22] <cjwatson> which is pretty clear
[00:23] <cjwatson> it's fairly grotty code but just stick to analysing the bits you need and it's ok
[00:24] <penguin42> 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] <cjwatson> 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] <cjwatson> although leaving it there wouldn't actually hurt anything
[00:26] <penguin42> yeuch
[00:26] <juliohm> penguin42, thank you very much for the link, it's really helpful. ;-)
[00:26] <juliohm> I'll try the process when i have time.
[00:27] <penguin42> cjwatson: The code there comes from one of the debian patches
[00:27] <penguin42> cjwatson: watch_unicode.patch
[00:27] <cjwatson> yep
[00:28] <cjwatson> is that a big deal?
[00:28] <penguin42> I was just wondering if it was a debian patch or an ubuntu patch, but I think it's a debian one
[00:29] <cjwatson> looks like it
[00:29] <penguin42> nod
[00:30] <penguin42> cjwatson: Is the right thing there to add another patch to the end of the set or to fix that patch?
[00:31] <cjwatson> judgement call in general - in this case I'd probably fix that patch
[00:31] <penguin42> ode for a 1 character tweek
[00:31] <cjwatson> though I wouldn't reject a branch that added another one, I guess
[00:31] <cjwatson> but yeah, seems overkill
[00:32] <penguin42> actually it needs one of the other patches tweeking as well because that is diff'd against it
[00:33] <cjwatson> quilt push; quilt refresh your way up the stack
[00:33] <cjwatson> but basically trivial
[00:35] <penguin42> 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] <cjwatson> 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] <cjwatson> but you might want the learning experience and if so I don't want to deprive you either :)
[00:37] <penguin42> I'll look tomorrow, or should I say at 1:37 am later today
[00:43] <penguin42> right, bed!
[06:52] <YokoZar> https://launchpad.net/ubuntu/precise/+queue  :D
[06:54] <infinity> YokoZar: Yeah, I'm trying to decide if that was a joke. :P
[06:54] <infinity> YokoZar: Do I review it or reject it?
[06:54] <YokoZar> infinity: Review it of course, it fulfills its stated purpose ;)
[06:54] <infinity> Oh dear...
[06:55] <infinity> Oh, derp.  April 1.
[06:58]  * micahg is curious if a second person signed off on it
[07:04] <jhojho> 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] <cjwatson> jhojho: no, I haven't got round to talking with the Debian maintainer about it yet
[09:23] <Whoopie> cjwatson: Hi, could you perhaps sponsor another package?  -> mail-notification, bug 881039
[09:24] <cjwatson> Whoopie: not right now, need to go out
[09:25] <Whoopie> cjwatson: ok
[09:25] <Whoopie> have fun
[09:25] <cjwatson> I can look later if nobody beats me to it
[09:30] <Daviey> Whoopie: you might find that some sponsors are unwilling to upload, without a Firstname Lastname in debian/changelog.
[09:31] <Whoopie> Daviey: might be. Even though I see a lot of packages with the same "issue".
[10:06] <infinity> Whoopie: It's not terribly common for people to upload with a real name.
[10:06] <infinity> Whoopie: But if the code's good, I don't care deeply either.
[11:18] <nigelb> ZarroBoogs: awww <3
[15:24] <PaoloRotolo> Hi all!
[16:20] <hallyn> 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] <hallyn> (maybe I mis-read a subject and it was another xserver-xorg...)
[16:35] <tjaalton> 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] <hallyn> 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] <hallyn> tjaalton: if it doesn't explicitly fix a bug, then you might be right we should file ffe.
[16:37] <hallyn> guess it coudln't hurt to file ffe anyway, and then comment there if it fixes a bug
[16:38] <hallyn> (really, spice should be seen as a tech preview right now)
[16:40] <tjaalton> hallyn: I don't see any bugs open against -qxl, so this was something reported on irc?
[16:40] <hallyn> tjaalton: bug 913314
[16:41] <hallyn> after confirmation of the fix of original bug, he lists a few other things still going wrong.
[16:41] <tjaalton> ah, ok
[16:44]  * penguin42 looks up
[18:05] <JamesJRH> Is it possible for me to obtain a correct source of TAI from my system if I keep it in sync?
[18:06] <JamesJRH> 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.
[19:19] <mamemame187> hello