--- Log for 19.07.121  Server: molybdenum.libera.chat  Channel: #rockbox
--- Nick: rb-logbot  Version: Dancer V4.16  Started: 4 days and 22 hours ago

00.01.27 ***     Saving seen data "./dancer.seen"
01.46.34 Join    Maxdamantus [0] (~Maxdamant@user/maxdamantus)
01.50.24 #       <Arsen> speachy: four ports on the same controller fail in different way and various other devices wrok, though, and same cable works on other places
01.51.05 #       <Arsen> fron 3.0 ports fail after a while, resetting, 2.0 ones never figure out configuration
02.01.28 ***     No seen item changed, no save performed.
02.50.26 Quit    Maxdamantus (Ping timeout: 256 seconds)
02.51.46 #       <desowin> Arsen: can you share pcap? do you have OpenVizsla or similar?
02.52.39 #       <Arsen> no I'm not usually working on this part of the kernel 
02.53.24 #       <Arsen> I can share the usbmon capture thon
02.53.41 #       <Arsen> after i sanitize it to see whether any unrelated and potentially sensitive data is there 
03.05.36 #       <desowin> you mean actual data stored on the drive or other devices?
03.06.32 #       <desowin> if the latter, then simply filtering for device number and saving filtered packets should do
03.07.25 Join    Maxdamantus [0] (~Maxdamant@user/maxdamantus)
03.08.24 Quit    akaWolf (Ping timeout: 252 seconds)
03.10.17 Join    akaWolf [0] (~akaWolf@akawolf.org)
03.33.28 #       <Arsen> both really 
04.01.32 ***     Saving seen data "./dancer.seen"
05.43.34 #       <Arsen> did anyone try implementing progressive jpegs or was there just no interest in that topic? I don't know why it's more complicated compared to baseline, if it even is, so I figured I should ask before looking into it
05.45.26 #       <gevaerts> I don't know *any* details, but Unhelpful (who did the work) has said at some point that progressive is not possible in the same efficient way the rest works
05.46.50 #       <gevaerts> Our jpeg decoder does decoding and scaling at the same time and doesn't need more RAM for bigger images (you can show 20 megapixel jpegs on your lower-end rockbox device if you really want to, although that tends to be slow), which is essential for our use
05.49.01 #       <Arsen> afaik the point of progressive jpegs is for data to be separated out such that it slowly increases in resolution as you proceed through the file, maybe it's possible to end early? - an uneducated guess
05.51.12 #       <gevaerts> I have no idea :)
05.54.13 #       <gevaerts> I'll find the logs where he explained it
05.56.16 #       <gevaerts> https://www.rockbox.org/irc/log-20090502#02:42:31 and https://www.rockbox.org/irc/log-20090514#08:46:06
05.56.54 #       <gevaerts> Note that I don't really understand most of that
05.57.31 #       <gevaerts> All I can do is find links :)
05.58.24 #       <Arsen> ah, this makes sense
05.59.51 #       <Arsen> desowin: alright, I read the entire ptrace, it should be clean, where would you prefer me to upload it for you, if you have a preference?
05.59.58 #       <Arsen> s/ptrace/pcap/
06.01.35 ***     No seen item changed, no save performed.
06.05.17 #       <Arsen> desowin: if you don't mind it being on my own webserver, here you go: https://www.aarsen.me/~arsen/failed_flac_write.pcapng
06.31.21 Quit    akaWolf (Ping timeout: 258 seconds)
06.46.25 #       <braewoods> Arsen: you want assistance submitting a quirks patch?
06.46.51 #       <braewoods> i've done this before but i'd need to know what the quirk is you're needing patched
06.47.13 #       <Arsen> yeah, I don't yet, I'll investigate further
06.47.23 #       <Arsen> if you have notes/docs about the process, I'm all ears though
06.47.36 #       <braewoods> depends, i've mainly done audio quirks.
06.47.42 #       <braewoods> and depends where it's required.
06.48.03 #       <braewoods> if it requires patching the kernel, you'll need to add hardware IDs to the in kernel driver table.
06.48.10 #       <braewoods> and maybe some other bits
06.48.46 #       <Arsen> probably the USB driver
06.48.52 #       <braewoods> if it's keyboard related that goes in the hardware database shipped with systemd
07.15.09 Join    akaWolf [0] (~akaWolf@akawolf.org)
07.51.27 Quit    kadoban (Quit: Bridge terminating on SIGTERM)
07.51.29 Quit    blbro[m] (Quit: Bridge terminating on SIGTERM)
07.54.04 Join    kadoban [0] (~kadoban@user/kadoban)
08.00.47 Join    blbro[m] [0] (~blbrostra@2001:470:69fc:105::8f7)
08.01.36 ***     Saving seen data "./dancer.seen"
08.08.13 Join    mixfix41 [0] (~user@user/mixfix41)
08.20.16 Join    cockroach [0] (~blattodea@user/cockroach)
08.44.41 #       <Arsen> braewoods: no, it's most likely either usbms or xhci_hid in the kernel
09.04.23 Quit    F3l1x_10m (Read error: Connection reset by peer)
09.07.22 Join    massiveH [0] (~massiveH@ool-18e4e82f.dyn.optonline.net)
09.37.46 Quit    Maxdamantus (Ping timeout: 268 seconds)
09.44.14 #       <braewoods> this is a warning i've never seen before
09.44.17 #       <braewoods> warning: comparison of promoted ~unsigned with unsigned
09.44.28 #       <desowin> Arsen: hardware level capture would tell a lot here, it didn't manage to send whole payload (240 * 512) but the length it managed to send (99 * 512)
09.44.48 #       <desowin> do you have corresponding logf?
09.46.05 #       <braewoods> oh well, converted it to xor equivalent
09.46.07 #       <desowin> if you enable ramdisk you can try to determine if the iflash has any impact on it
09.46.30 #       <Arsen> I don't think I have it, lemme look through some logs
09.46.44 #       <desowin> as ramdisk will simply present a chunk of ram as the disk to the host
09.46.53 #       <Arsen> and it almost certainly doesn't at this point, since a different computer works
09.47.29 #       <Arsen> different controllers in this PC even work
09.47.39 #       <desowin> the port reset is just a consequence of OUT URB timing out
09.47.45 #       <Arsen> nope, lost the logs, rip
09.47.47 #       <Arsen> yeah, thought so
09.48.16 #       <desowin> so there's either a signal integrity issue (hardware packet level capture would tell if that's the case) or something in software
09.48.29 #       <braewoods> a little closer to finishing inflate support
09.48.37 #       <braewoods> next i need to add the huffman engine
09.49.17 #       <Arsen> `logf("transfer result for ep %d/%d %X %d", ep,dir,status, length);` this log was triggered followed by a scsi write10 0, I think
09.49.42 #       <Arsen> in usb_storage.c
09.49.47 #       <Arsen> I'm not sure I saw the write10 at the end
09.49.57 #       <Arsen> I'll get another debug build
09.50.43 #       <Arsen> desowin: how do you know it didn't manage to send the full payload?
09.51.02 #       <desowin> it didn't manage to send the full payload
09.51.24 #       <desowin> it got 99 packets (out of 240) through
09.51.27 #       <Arsen> yes, but what indicates that?
09.51.38 #       <Arsen> oh, where are those numbers from?
09.51.56 #       <desowin> URB length in the "packet" when the URB returns from host controller
09.52.08 #       <Arsen> the ECONNRESET one?
09.52.11 #       <desowin> yes
09.52.13 #       <Arsen> ah
09.53.55 #       <Arsen> what files should I put the logfs into?
10.01.37 #       <desowin> firmware/drivers/usb-designware.c and firmware/usbstack/usb_storage.c
10.01.39 ***     Saving seen data "./dancer.seen"
10.01.53 #       <Arsen> presumably USB serial logging won't work for this? :^)
10.02.24 #       <desowin> you can have it compiled in, but don't enable to not distort the result
10.02.30 #       <Arsen> yeah
10.02.35 #       <desowin> you can simply dump log to file
10.02.42 #       <Arsen> oh, right, that works
10.02.47 #       <Arsen> after I fix the FS that is :P
10.03.13 #       <desowin> FS meaning file system or full speed USB?
10.03.19 #       <Arsen> filesystem
10.06.07 #       <desowin> capture pcap as well to have the logf and pcap from same session
10.06.22 #       <Arsen> sure, I'll just transfer on the debug build now
10.06.56 #       <Arsen> sorry if I take some time, I'm dealing with some other issue as well, unrelated to CF
10.20.32 Quit    massiveH (Quit: Leaving)
10.36.18 Quit    spork (Ping timeout: 255 seconds)
11.19.26 #       <Arsen> right, I'm back, sorry about that
11.43.12 #       <Arsen> alright, I can see logs, lets see what happens now
11.47.04 #       <Arsen> desowin: got the logs and stuff
11.49.41 #       <Arsen> this buffer is tiny
11.49.47 #       <Arsen> I'm not sure if I have the right bit there
11.49.55 #       <Arsen> most likely not, I'll redo this
11.50.09 #       <Arsen> issue is, I need(?) to fsck before the dump
11.50.19 #       <Arsen> I'll try again since I can't corelate it myself either
11.55.07 #       <Arsen> ok I can corelate it this time
11.55.21 #       <Arsen> desowin: .tar of the two on the same place works for you?
12.01.43 ***     Saving seen data "./dancer.seen"
12.10.45 #       <Arsen> https://aarsen.me/~arsen/20210719_1737_usbstuff.tar.zst
12.32.10 Join    spork [0] (topic@31-151-2-135.dynamic.upc.nl)
12.38.51 Join    ZincAlloy [0] (~Adium@ip5f5abcae.dynamic.kabel-deutschland.de)
12.42.56 #       <desowin> I don't quite know this usb (device) chip, so not really much I can do here, but looking at the logs I would be suspicious about Rockbox USB driver (not the stack)
12.43.11 Quit    ZincAlloy (Ping timeout: 258 seconds)
12.44.21 #       <desowin> depending how much you want to debug, if I was debugging this, I would add logf to usb_drv functions, especially usb_drv_recv to see if it is called before the failure occurs (I assume yes, but you never know)
12.48.32 #       <desowin> getting 3 packets on top of the successfully received two 24 KiB chunks is suspicious
12.49.17 Join    ZincAlloy [0] (~Adium@2a02:8108:943f:d824:ca6:7e8e:19f4:c808)
12.53.44 Quit    ZincAlloy (Ping timeout: 255 seconds)
13.00.43 Join    lebellium [0] (~lebellium@2a01:cb10:2e:2000:5063:e165:3a35:2673)
13.19.14 Join    ZincAlloy [0] (~Adium@2a02:8108:943f:d824:f5db:f084:d263:e397)
13.25.13 #       <Arsen> desowin: you saw that in the logf?
13.25.22 #       <Arsen> I'd like to debug this honestly, I've got the time right now
13.26.27 #       <Arsen> I don't see the three packets though
13.29.20 #       <desowin> URB finished writing 50688 bytes, but there were two times transfer result for ep 2/0 0 24576, the difference is three packets (high speed maximum packet size is 512)
13.29.50 #       <desowin> host can only write in multiples of maximum packet size, unless the packet ends transfer (1 URB = 1 transfer)
13.31.25 #       <desowin> the device acknowledged these three packets, and stored them *somewhere*
13.32.39 #       <desowin> next ones were most likely NAKed (most liekly PINGs were NAKed, not the actual OUT transactions), but to know for sure you'd need hardware level sniffer
13.32.55 #       <Arsen> after the NAK the timeout happens?
13.33.30 #       <desowin> there are thousands of NAKs
13.33.56 #       <desowin> the ECONNRESET "packet" in Wireshark is 30 seconds after the URB submit
13.34.05 #       <desowin> so host was trying to send the data for 30 seconds
13.34.16 #       <Arsen> and getting nak'd the entire time?
13.34.38 #       <Arsen> so, usbmon actually logs requests for something to happen on usb, not what's actually happening on usb?
13.34.48 #       <desowin> not entire time, it got ACKed 99 times (50688 / 512)
13.34.49 #       <Arsen> (and their results)
13.35.21 #       <Arsen> ah
13.35.48 #       <desowin> yes, usbmon logs requests for something to happen on usb bus
13.36.00 #       <Arsen> right, that makes a lot of sense
13.36.18 #       <desowin> https://www.youtube.com/watch?v=cUljKImph4s this is recording of my SharkFest '20 Virtual talk on USB
13.36.46 #       <Arsen> how much is that sniffer you mentioned earlier?
13.36.50 #       <Arsen> as a side note
13.36.57 #       <Arsen> (the cost of acquiring it)
13.37.55 #       <desowin> explains what usbmon captures and how it differs from the packets on the bus, nowadays if you get https://gitlab.com/wireshark/wireshark/-/merge_requests/2848 the usbll capture will be actually reassembled
13.38.07 #       <desowin> for sharkfest I just correlated the packets manually
13.38.16 #       <Arsen> I could pull in usbll if it can help here
13.38.36 #       <Arsen> this patch*
13.38.37 #       <Arsen> not usbll
13.39.06 #       <desowin> the patch only affects usbll captures, so no change for usbmon capture
13.39.16 #       <Arsen> ah, and that requires some device?
13.39.33 #       <desowin> http://shop.sysmocom.de/products/openvizsla-v3-dot-2-usb-protocol-analyzer-pcba has assembled OpenVizsla for 154.70 EUR + shipping
13.39.54 #       <desowin> but mind you, just getting usbll capture is only a beginning of actual troubleshooting
13.40.39 #       <desowin> but it can for sure keep you from wandering entirely wrong direction as it actually shows what happens on the bus
13.41.11 #       <Arsen> yeah
13.45.37 #       <Arsen> that number is oddly specific, though
13.45.43 #       <Arsen> 99, and I think that was happening before too
13.46.00 #       <desowin> 99 not so much, as that's just 2 * 24 KiB buffer + 3
13.46.13 #       <Arsen> yeah, but it's a bit oddly specific
13.46.26 #       <desowin> the 3 sounds like what the chipset might be able to acknowledge "on its own" before driver has to read it
13.47.06 #       <desowin> atleast that's a guess from someone who has dealt with USB in general, but not particularly this chip
13.48.39 #       <Arsen> I'll enable ramdisk too
14.01.44 ***     Saving seen data "./dancer.seen"
14.03.49 #       <Arsen> desowin: I don't think I can reproduce it writing 8MiB per attempt to the ramdisk
14.04.09 #       <Arsen> that's also significantly less than the failing writes
14.04.25 #       <Arsen> also, if this is due to the dwc driver, why does it not happen on reads?
14.05.48 #       <Arsen> wait wrong controller
14.11.37 #       <Arsen> while :; do yes AAAAAAAAAAAAAA | doas dd of=/dev/sde; done I'm doing this to test
14.13.48 #       <desowin> I wouldn't reject the possibility of it being in dwc driver just on that, but yes, it can be storage
14.15.08 #       <desowin> if you try to find the problem not where it is, then the debugging takes really long time
14.15.55 #       <desowin> but here, I would suspect both, atleast not until I am perfectly sure that one is perfectly fine and not affecting the other at all
14.18.38 #       <desowin> speachy: should we register for Coverity Scan? in my experience it gives really good results
14.20.36 #       <desowin> as far as compilation goes you just run make through cov-build and then upload compressed cov-int directory
14.23.26 #       <Arsen> well, I have dismissed storage before because 1) it works fine on other controller (in fact, I synced my entire library onto it, but on this controller it fails with one flac) and 2) it works fine if I just copy files in rockbox' file manager
14.24.24 #       <desowin> speachy: we might not be able to get it for all builds due to limits https://scan.coverity.com/faq#frequency but it would be really nice to get it going
14.24.26 #       <Arsen> when I say controller, I usually mean USB host controller
14.28.13 #       <speachy> desowin: I've not been motivated to deal with the tooling changes; IIRC they don't do so well with cross-complied crap.  though of course the code that makes it into a sim build would be a pretty good subset of our codebase.
14.29.48 #       <desowin> I got good results with cross compiled code for Nintendo switch
14.29.51 #       <speachy> (it's not as simple as overriding CC/LD/etc. I've done a lot to detangle things (as part of the gcc494 transition) but plenty of work remains
14.30.52 #       <Arsen> wtf, echo test | dd of=/dev/sde causes it to requery the serial code and such
14.30.58 #       <desowin> speachy: I would say it is comparably simple to overriding CC, the problem is with the upload frequency
14.30.59 #       <Arsen> (rerequest descriptors in general)
14.31.33 #       <speachy> (no, I mean we can't just override CC and have it proprogate everywhere successfully yet)
14.31.42 #       <speachy> but we're at ~2.1M LOC.
14.32.01 #       <braewoods> it might help if we start dropping dead ports no one has touched in ages
14.32.08 #       <speachy> not really. 
14.32.14 #       <braewoods> oh?
14.32.15 #       <speachy> we'd be limited to 1 build per day, period
14.32.27 #       <speachy> unless we pay extra, granted
14.32.31 #       <braewoods> oh you mean coverity
14.32.44 #       <desowin> speachy: if you don't mind, I can request account and upload Sansa Connect build there to check it out
14.33.04 #       <speachy> most of the code is not in ports, but in plugins/library code.
14.33.21 #       <desowin> that's shared between all targets, so good
14.33.34 #       <speachy> desowin:  Not that you need permission to do that, but by all means, please do
14.34.08 #       <desowin> Arsen: does it actually trigger USB reset? why would it rerequest descriptors otherwise?
14.34.41 #       <Arsen> no, no reset happens, and it keeps normally sending Test Unit Ready commands
14.35.04 #       <Arsen> wait, /dev/sde isn't a device
14.35.09 #       <desowin> basically, device can only reply to host, there's no way any code on device would result in host rereading descriptors
14.35.17 #       <speachy> well, I suppose if you have to "officially" register it that's another matter.
14.35.33 #       <desowin> I would only think about resets and other recovery actions performed by host
14.35.55 #       <Arsen> /dev/sde wasn't even a node in devfs, I unplugged it and plugged it back in, it is now
14.36.01 #       <desowin> you have to be contributor, which I fulfill
14.36.35 #       <speachy> desowin: the reason I think we should stick to simulator builds for the first cut is that it uses stock modern toolchains
14.37.04 #       <speachy> ie no fancy linker scripts, asm startup code, or so forth.
14.37.32 #       <speachy> and, heh, I'm 99% sure that will end up with several hundred (or so) issues in the codecs alone.  :)
14.37.35 Join    saratoga [0] (~saratoga@cpe-98-10-205-66.rochester.res.rr.com)
14.38.19 #       <saratoga> Arsen:  I never looked at it, but I think progressive jpeg could be supported by only decoding one layer of the progressive image and then rescaling that
14.38.33 #       <desowin> yes, it will, Wireshark has a lot of outstanding issues reported by Coverity Scan but it helps detecting problems in new code pretty nicely
14.38.34 #       <braewoods> woops. i had my shift register's bits reversed :D
14.38.38 #       <saratoga> but i'm not sure how easy that would be to do, i didn't look at how they're packed in progressive mode
14.38.38 #       <Arsen> saratoga: that's what I was thinking as an implementation too fwiw
14.38.53 #       <Arsen> but according to the logs, that's also impossible due to the MCU being huge
14.39.24 #       <Arsen> as in, you'd need the whole image decoded to start scaling it, and that would take up a lot  of mem
14.39.48 #       <Arsen> it may be possible on some more modern devices, this ipod has 64M of RAM, I have no idea whether that's enough until I try, though
14.39.50 #       <speachy> saratoga: I think to handle progressive we'd have to not scale as we decompress because each iteration builds on the previous.  I don't know if we can scale on the fly any more.
14.40.03 #       <Arsen> ^ you couldn't
14.40.21 #       <Arsen> which is why I was thinking of just giving up at reading further at some point
14.40.59 #       <saratoga> i think he means decoding it at full resolution
14.41.05 #       <Arsen> yes
14.41.51 #       <saratoga> i think at least you could decode the lowest resolution step (1/8th) efficiently
14.42.01 #       <saratoga> although it would only be useful for large images
14.42.53 #       <saratoga> probably not worth it for such niche use
14.44.16 #       <desowin> ok, registered Rockbox on Coverity Scan, will submit build soonish
14.44.59 #       <speachy> desowin: another option is to enable support for clang's scan-analyze builds.
14.46.01 #       <desowin> I wouldn't compare these two, Coverity Scan is just another league IMHO
14.46.24 #       <Arsen> what does coveritry do for rockbox?
14.46.27 #       <speachy> yes, but it has the advantage of not requiring 3rd party integration and access/service limitations.
14.46.32 #       <braewoods> Arsen: static code analysis
14.46.38 #       <Arsen> ah, I see
14.46.38 #       <braewoods> it can find issues that compilers miss
14.46.42 #       <Arsen> clang-tidy?
14.46.48 #       <speachy> I've used clang's stuff with great effect for other projects of mine
14.47.11 #       <desowin> I got USBPcap blue screen of death root cause pointed out by Coverity Scan
14.47.13 #       <speachy> (and speaking of coverity, at my last $dayjob we had a _lot_ of false positives too)
14.47.19 #       <desowin> that I wouldn't otherwise have found
14.47.47 #       <bertrik> I use cppcheck for a quick check a lot
14.48.25 #       <bertrik> it doesn't do a very deep analysis, but can quickly find the obvious stuff with barely any configuration
14.48.59 #       <desowin> at work I have been involved in projects using Klocwork, it was funny that people suspected that there's something wrong with the tool as my code had just 2 issues found
14.49.08 #       <Arsen> alright, I've got some python going to spam the ramdisk with 8MB of "B" characters
14.49.10 #       <speachy> I've had some success with cppcheck too, but it's pretty superficial.  Doesn't do much better than simply building with all warnings turned on.
14.49.35 #       <desowin> but well, it wasn't anything big, just simple measurements and valve control
14.50.16 #       <speachy> desowin: heh, yeah, I'm always suspicious when I see initial runs of sanity checks turn up no problems.  :D
14.50.50 #       <bertrik> code at work is sometimes very well tested (high coverage, etc) but we forget to see if it does what it's supposed to do for the customer
14.51.22 #       <braewoods> speachy: i've introduced myself to two different constructs i've never needed before i worked on inflate... a circular output buffer and a bit reader layered on top of a byte stream
14.51.23 #       <speachy> (BTW I still have a scan-build going on the X3 simulator.  mostly dead stores, a few divide-by-zeros so far)
14.51.28 #       <braewoods> kinda interesting
14.51.46 #       <Arsen> desowin: yeah, spamming it with writes isn't doing anything, I think
14.55.33 #       <speachy> lots of thjings flagged in the codecs so far, mostly due to shifts.
14.58.10 #       <Arsen> speachy: I have figured out why there aren't more people complaining: this *only* happens with *one* USB controller, and not even all ports on it, and only while writing to the disk from the usbms driver
14.58.57 #       <speachy> I assume the physical ports can be plugged into a different set of motherboard headers?
14.59.14 #       <Arsen> it's not the controller and the cable to it or to the ipod, though, because the same cables work fine if any of those variables is different (using a ramdisk, or for reading)
14.59.37 #       <desowin> well, personally I would only blame it on the controller if I actually checked the "real" USB packets
14.59.57 #       <Arsen> yeah, if I get together enough money for that sniffer I might get it
14.59.58 #       <speachy> I mean, it still reeks of a phyiscal hardware issue (eg bad port/cable to the motherboard)
15.00.06 #       <desowin> there might be some subtle out-of-bounds write that triggers only in this condition
15.00.32 #       <speachy> but without seeing true wire-level sniffs (ie showing CRCs) 
15.00.53 #       <desowin> with timing just right in case of this particular controller
15.00.54 #       <Arsen> at this point I am more curious about what's wrong here than actually caring to fix it in order to use it, because changing any of those variables makes it "work", so I might put together some money to buy a sniffer
15.01.01 #       <speachy> logically the packets are identical; if port A vs port B of the same controller behaves differently.
15.10.23 #       <speachy> ok, in the X3 sim build, scan-build found 358 bugs.
15.10.55 #       <bertrik> nice
15.11.06 #       <speachy> 159 of those are dead stores.
15.12.21 #       <speachy> 55 nullptr derefs, 14 div0.. and a bunch of uninitialized/undefined values used where it matters.
15.14.02 #       <desowin> I go with Sansa Connect target build as first Coverity Scan submission due to selfish reasons - just curious if I have some strange problems in drivers
15.14.47 #       <desowin> no idea really how many issues there will be
15.16.00 #       <desowin> but it gets all the codecs and plugins so the results will be to great degree valid for other targets (possibly to all, not only armv4)
15.16.51 #       <speachy> some of those are data-depenent; eg impossible to trigger unless we have no language files.
15.17.06 #       <speachy> (...)
15.17.57 #       <braewoods> incidently null pointer dereference isn't necessarily an error
15.18.10 #       <speachy> (but it almost always is)
15.18.23 #       <braewoods> indeed, one place you could ignore it if it pops up is iriver_flash
15.18.30 #       <braewoods> it's going to be a false positive there
15.18.48 #       <speachy> because address 0 is a valid address (which is actually a violation of the C standard)
15.19.11 #       <braewoods> yes, but the real world doesn't always care what the C standard says. :|
15.19.25 #       <speachy> lots of nullptr issues int he skin code
15.19.33 #       <braewoods> technically you're not supposed to assume char is 8 bits yet people do it anyway
15.20.12 #       <braewoods> but in practice only some esoteric legacy systems ever had a different bit value assigned to char
15.20.18 #       <Arsen> oh, this looks interesting "[366912.197535] sd 11:0:0:0: [sdf] Synchronize Cache(10) failed: Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK"
15.20.22 #       <Arsen> when did this happen
15.21.28 #       <Arsen> 11 minutes ago, that was after I stopped spamming it with writes, that's probably not related to this issue then
15.22.51 #       <speachy> the '-1' in this:  #define TALK_ID(n,u) (((long)(u))<<UNIT_SHIFT | ((n) & ~(-1L<<DECIMAL_SHIFT)))
15.23.01 #       <speachy> is responsible for a substantial portion of the errors.
15.23.15 #       <speachy> since left-shifting a negative number is undefined.  
15.23.48 #       <speachy> but I suppose casting that -1 to (unsigned int) will probably shut things up
15.24.26 #       <braewoods> speachy: add a U?
15.24.37 #       <braewoods> or will  that even matter here
15.24.39 #       <speachy> -1UL is a contradiction.  :)
15.24.46 #       <braewoods> even so it may still work
15.24.59 #       <braewoods> i rarely need to use the U suffix though
15.25.19 #       <braewoods> most of the time 'int' default integer constant is enough as it gets promoted to the proper type
15.25.28 #       <braewoods> or demoted
15.26.11 #       <speachy> we're relying on a specific hardware numeric representation (ie 2's complement) that C doesn't promise.
15.26.18 #       <speachy> hence the warning
15.26.19 #       <braewoods> huh, IS_ALIGNED is one weird macro. i assumed it would cast pointer to an integer
15.26.27 #       <braewoods> but no such luck
15.26.39 #       <braewoods> since the main use for it is checking pointer alignment
15.29.33 #       <speachy> ok, fixing that knocked out 12 "errors"
15.33.21 #       <desowin> anyone who wants to see Rockbox Coverity Scan project, please let me know their email so I can send invite
15.33.30 #       <speachy> send one to me, please
15.33.47 #       <desowin> the results will be available only after the application is reviewed (1-2 business days)
15.35.37 #       <Arsen> I'd like it too
15.35.44 #       <Arsen> out of curiosity, not necessity, if it's no issue
15.36.08 #       <desowin> Arsen: pm me your email
15.36.33 #       <desowin> speachy: invitation should be sent, but I can't see it in members page, so not sure
15.36.48 #       <speachy> thank you
15.38.48 #       <desowin> ok, you have to press space after parting in the email and click on the "add email" button
15.39.00 #       <desowin> as otherwise it was just submitting empty list
15.39.21 #       <desowin> now the invitations are visible as "not yet accepted"
15.40.36 #       <braewoods> g#3590
15.40.38 #       <rb-bluebot> 3Gerrit review #3590 at https://gerrit.rockbox.org/r/c/rockbox/+/3590 : 3system: revamp IS_ALIGNED macro by James Buren
15.40.57 #       <braewoods> \o/
15.41.51 #       <speachy> was it really only used in one file?
15.42.05 #       <braewoods> yea, oddly enough.
15.42.16 #       <braewoods> the only other hit is miknod files which has its own macros instead
15.42.20 #       <braewoods> mikmod
15.42.33 Join    Maxdamantus [0] (~Maxdamant@user/maxdamantus)
15.42.45 #       <braewoods> https://dpaste.com/A7YPBYM7W
15.43.20 #       <braewoods> i'd have preferred an inline function but i figured i should keep the macro solution.
15.44.55 #       <braewoods> speachy: if you're ok with that though i could replace it with an inline macro. in this case the types don't really matter
15.45.08 #       <braewoods> err
15.45.10 #       <braewoods> inline function
15.45.12 #       <braewoods> doh
15.45.28 #       <braewoods> i mainly like inline functions because they're usually easier to understand than macros
15.47.14 #       <speachy> as a macro is fine
15.47.31 #       <braewoods> ok
15.47.49 #       <speachy> the overhead of calling a function will probably always be greater than just inlining it anyway.
15.48.10 #       <braewoods> i just prefer inline functions as much as possible for simple stuff
15.48.32 #       <braewoods> they benefit from being easier to debug and the like
15.48.48 #       <braewoods> the only disadvantage is they can't workaround syntax limitations like macros can
15.48.52 #       <braewoods> that i can think of
15.49.07 #       <braewoods> sometimes i pair them together
15.49.17 #       <braewoods> inline function for the read code, macro is a wrapper
15.49.20 #       <braewoods> real*
15.50.12 #       <braewoods> i've also sometimes done hacks with extern inline
15.50.44 #       <braewoods> it's a weird duck. if you use it correctly you can have both an inline version and an external version so the compiler can choose both and you won't  have a million copies of the static version
15.52.32 Quit    Maxdamantus (Ping timeout: 252 seconds)
15.54.15 #       <braewoods> incidently we can now use _Alignof thanks to newer GCC
15.57.01 #       <speachy> _bilgus: here's an interesting one.  apps/gui/skin_engine/skin_render.c L324
15.57.45 #       <speachy> the call uses info->skin_vp instead of the skin_vp passed in as part of the function.  
15.59.02 #       <speachy> everyelse in the function the function-arg-supplied one is used.
16.01.45 ***     Saving seen data "./dancer.seen"
16.05.48 #       <braewoods> speachy: do we have any 64 bit ports other than some kind of hosted one?
16.05.54 #       <braewoods> i'm assuming no
16.06.07 #       <braewoods> i'm guessing even the x1000 isn't 64 bit
16.06.25 #       <speachy> most sim builds are 64-bit these days
16.06.39 #       <braewoods> indeed but in terms of native or hosted...
16.07.09 #       <braewoods> if we had a 64 bit native, we could look into 64 bit optimizations but for now not much point
16.07.17 #       <speachy> braewoods: btw, #3590, the old macro allows for varying degrees of alignment checking depending on the cast used.  the revamped only checks for pointer-sized alignment.
16.07.47 #       <braewoods> speachy: that was the whole idea? what else needs to be checked for alignment?
16.08.02 #       <speachy> it's a generic macro, so could be used for arbitrary alignment checks.
16.08.22 #       <speachy> (eg dma that might need cacheline-aligned stuff for example)
16.08.47 #       <braewoods> i have an idea. gimme a minute to research it.
16.09.01 #       <speachy> (or arm processors that need the irq vector table to be 128-byte aligned or somesuch)
16.09.49 #       <speachy> now IMO the right thing to do is make the type/size you want to align to an argument to the macro
16.10.18 #       <speachy> explicitly instead of implicitly.  IIRC Linux's alignment check macros work this way.
16.16.07 #       <braewoods> of course _Generic doesn't work like this...
16.16.24 #       <braewoods> i wish i could use it as a general catch all for pointers
16.16.53 #       <braewoods> speachy: in any case, what different does it make? pointer address integers are just as valid as any other, no?
16.17.19 #       <braewoods> though i guess in terms of semantics
16.17.29 #       <braewoods> they are different even if they are usually the same
16.17.35 #       <speachy> as in that new macro only works for pointers, the old one worked with whatever.
16.17.59 #       <braewoods> it cast to uintptr_t, which should work for regular integers too
16.18.14 #       <speachy> I'm talking about things larger than a pointer
16.18.21 #       <speachy> or smaller
16.18.30 #       <braewoods> larger is a problem, but smaller?
16.18.43 #       <braewoods> smaller gets promoted and should retains its value
16.19.25 #       <braewoods> but afaik for the most part sizeof(void*) == sizeof(largest integer type)
16.19.28 #       <speachy> call the macro IS_ALIGNED_PTR() to reflect what it really does
16.19.58 #       <speachy> because you're changing its semantics.
16.20.40 #       <speachy> what I'd like to see instead is a reduction of the various alighment checks/macros all over the codebase.
16.46.20 Quit    saratoga (Quit: Connection closed)
17.01.48 Quit    lebellium (Quit: Leaving)
18.01.49 ***     Saving seen data "./dancer.seen"
18.10.51 Quit    ZincAlloy (Quit: Leaving.)
18.11.56 Join    ZincAlloy [0] (~Adium@ip5f5abcae.dynamic.kabel-deutschland.de)
18.18.24 Quit    ZincAlloy (Quit: Leaving.)
20.01.50 ***     Saving seen data "./dancer.seen"
20.04.39 #       <_bilgus> speachy, ya got me on that no clue..
20.05.16 #       <speachy> I won't pretend to understand the semantics of the menu code
20.05.57 #       <_bilgus> I'm guessing its something v. specific bc I don't recognize the flag..
20.05.59 #       <speachy> But g#3591 does seem to be the correct thing.
20.06.01 #       <rb-bluebot> 3Gerrit review #3591 at https://gerrit.rockbox.org/r/c/rockbox/+/3591 : 3menu:  Fix a potential nullptr deference. by Solomon Peachy
20.06.15 #       <speachy> (a different unrelated thing)
20.06.52 #       <_bilgus> ah lol
20.09.23 #       <_bilgus> temp is referenced before that..
20.09.54 #       <_bilgus> ah but checked -- nm
20.10.07 #       <rb-bluebot> Build Server message: 3New build round started. Revision df37450f91, 303 builds, 8 clients.
20.12.00 #       <speachy> there's a second potential nullptr around L599, temp can be null before the switch and lead to much dereferencing.
20.12.06 #       <speachy> in menu.c
20.12.28 #       <_bilgus> 495?
20.14.31 #       <speachy> starting at L587, if (in_stringlist) {} else if (temp) {}  and then switch(type) { ... } where every case dereferences temp.
20.15.06 #       <speachy> might only be triggerable if the menu structure is mis-defined
20.15.20 #       <speachy> not familiar with this stuff to know its subtelties.
20.18.23 #       <_bilgus> that one is guarded further up IIRC
20.18.43 #       <_bilgus> I think it was an oversight originally
20.19.01 #       <_bilgus> but L496                     const struct settings_list *setting =
20.19.01 #       <_bilgus>                             find_setting(temp->variable, NULL);
20.19.30 #       <speachy> L496 doesn't kick in if type == MT_MENU
20.19.32 #       <_bilgus> I don't see anything checking for validity but it might too be guarded]
20.20.20 #       <speachy> L476, actually
20.21.10 #       <_bilgus> huh how does that work??
20.21.19 #       <speachy> anyway, that's a different issue.  what I'm referring to is L575 on
20.21.44 #       <speachy> temp is assigned in there, only if type == MT_MENU, if it's not, then it's not guaranteed to be assigned
20.22.34 #       <speachy> so the MT_FUNCTION_CALL case dereferences default-assigned temp, ie NULL
20.22.54 #       <speachy> when action == ACTION_STD_OK
20.23.10 #       <rb-bluebot> Build Server message: 3Build round completed after 782 seconds.
20.23.13 #       <rb-bluebot> Build Server message: 3Revision df37450f91 result: All green
20.23.42 #       <speachy> it's possible that it's semantically impossible to call STD_OK if it's not a type MT_MENU
20.24.16 #       <_bilgus> seems unlikely
20.24.35 #       <speachy> not something I'd care to put money on either.  :)
20.27.01 #       <speachy> I'll put up the results of this scan-build pass and you can interactively see for yourself.
20.31.00 #       <speachy> _bilgus: https://www.shaftnet.org/~pizza/scan-build-202453/index.html
20.35.18 #       <_bilgus> I see the one at L184 and a dead assignment
20.40.56 Join    dconrad [0] (~dconrad@208.38.228.17)
20.41.32 #       <_bilgus> I guess this encourages you to add pointer null checks but most of these are likely benign
20.47.51 #       <speachy> yeah.  but there's a good chance there's some real bugs in there amongst the noise.
20.50.20 #       <_bilgus> indeed
20.51.09 #       <_bilgus> this one likely is https://www.shaftnet.org/~pizza/scan-build-202453/report-88af48.html#EndPath
20.52.06 #       <braewoods> oh man i hate parsers sometimes.
20.52.12 #       <braewoods> they seem like the ones most prone to bugs
20.52.28 #       <braewoods> sometimes i wonder if a parser generator would be a better idea
20.53.08 #       <speachy> braewoods: they usually are.
20.54.46 #       <_bilgus> https://www.shaftnet.org/~pizza/scan-build-202453/report-f9a6b6.html#EndPath
20.54.58 #       <_bilgus> that one is real
20.54.58 #       <braewoods> would it even be worth replacing the theme/skin parser?
20.55.00 #       <braewoods> at this point
20.55.13 #       <braewoods> i once dabbled in parsers
20.55.44 #       <braewoods> i'll look at the parser sometime and see what its hard requirements are
20.55.56 #       <braewoods> the grammar determines what kind of generators are practical
20.56.01 #       <speachy> _bilgus: yes but only if the language file is toast.
20.56.39 #       <speachy> ie num_clips == 0
20.57.54 #       <_bilgus> if (!size) continue;
20.58.22 #       <speachy> I suppose the correct thing to do is, after the loop is finished, bail immediately if real_clips is 0.
20.58.30 #       <braewoods> depending on requirements, i've usually used lemon for my grammar side.
20.58.46 #       <braewoods> re2c is also a decent choice
20.58.50 #       <braewoods> for lexer
20.59.58 #       <braewoods> one reason not to use bison... it's non-reentrant last i heard
21.00.15 #       <braewoods> a reentrant parser combo is probably a good idea these days
21.00.54 #       <speachy> braewoods: first you need a formal grammar, which IIRC doesn't actually exist.
21.16.09 #       <braewoods> yea,
21.16.13 #       <braewoods> i'd have to look
21.26.25 Join    Maxdamantus [0] (~Maxdamant@user/maxdamantus)
21.30.48 #       <_bilgus> !g3593
21.31.18 #       <_bilgus> g#3593
21.31.20 #       <rb-bluebot> 3Gerrit review #3593 at https://gerrit.rockbox.org/r/c/rockbox/+/3593 : 3talk.c check for 0 talk clips & announce_status fix typo by William Wilgus
21.33.17 #       <speachy> yay, first actual bug squashed.
21.33.41 #       <speachy> (required malformed input files, but hey, it's at least possible..)
21.34.40 #       <_bilgus> too bad I neglected a parens and introduced a far worse bug
21.34.44 #       <_bilgus> :p
21.35.54 #       <braewoods> _bilgus: it attained sentience?
21.36.49 #       <_bilgus> if it were that easy..
21.36.58 #       <speachy> is it really a new bug if it wouldn't even compile?
21.42.47 #       <_bilgus> https://www.shaftnet.org/~pizza/scan-build-202453/report-2f91fc.html#EndPath
21.43.11 #       <_bilgus> that one is a bug but I can't tell if its a real bug
21.47.43 #       <speachy> I think it doesn't matter what exp is initialized to... though 1 seems the logical default exponent
22.01.51 ***     No seen item changed, no save performed.
22.08.17 Quit    dconrad (Read error: Connection reset by peer)
22.08.49 Join    dconrad [0] (~dconrad@208.38.228.17)
22.22.42 Quit    mixfix41 (Ping timeout: 240 seconds)
22.31.51 Quit    cockroach (Quit: leaving)
22.50.38 #       <munkis> huh that's a strange construction
22.56.00 #       <munkis> The only reason I can think of to do that is to avoid compiler warnings while intentionally not initializing
23.23.28 Quit    dconrad ()
23.25.15 #       <_bilgus> whats that save 4 bytes?
23.49.45 #       <_bilgus> is this really a bug? https://www.shaftnet.org/~pizza/scan-build-202453/report-77fc0f.html#EndPath
23.50.44 #       <_bilgus> thoughts opinions diatribes?