| | <<<Back 1 day (to 2018/03/20) | 20180321 |
| tkamppeter | mvrhel_laptop, hi | 14:20.58 |
| sebras | kens: chrisl: HenryStiles looked a bit at these yesterday, but I'm not sure I got a full LGTM: I have resolved 698839 with four small commits on http://git.ghostscript.com/?p=user/sebras/ghostpdl.git;a=shortlog care to take a look? | 14:30.13 |
| kens | One momentsebr I can't see any problems, but I do not speak Python | 14:31.18 |
| | Probably best to get someone else to review | 14:31.26 |
| chrisl | sebras: LGTM | 14:33.10 |
| | (I'm assuming the checksum is correct, of course!) | 14:33.47 |
| sebras | chrisl: well, it doesnt print FAIL anymore, :) | 14:34.32 |
| | chrisl: I verified that the decoded output of amb_{1,2}.jb2 is identical to the one decoded by luratech. | 14:34.56 |
| chrisl | As Henry said, having to manually maintain the checksum is a pain, but I don't see much choice | 14:35.26 |
| sebras | chrisl: and the failed checksum is just the checksum of 0 bytes, which I verified by touch tomte; sha1sum tomte | 14:35.39 |
| | chrisl: the checksum of the _input_ data is questionable, but the checkout on the expected output is more reasonable. | 14:36.08 |
| | but what's probably best is to add all these files as PDFs to the cluster. | 14:36.27 |
| chrisl | *If* they are legal in PDF | 14:36.41 |
| sebras | because of licensing I would advice against adding these to the repo. | 14:36.52 |
| | chrisl: is jbig2 also restriced in some way inside PDF? I assumed that that only applied to jpeg2000. :-/ | 14:37.35 |
| chrisl | ISTR there are restrictions on jbig2 in PDF | 14:38.06 |
| kens | I thnk JBIG2 is OK in tha respect | 14:39.15 |
| chrisl | sebras: we could probably script up something that would periodically test and compare the checksums on casper, and ping out an e-mail if they change | 14:39.37 |
| tor8 | sebras: coverity threw up an uninitialized warning about one of your jbig2dec commits | 14:39.39 |
| kens | A new one ? We've had a few for a while in there | 14:40.52 |
| sebras | tor8: grr, link? | 14:40.53 |
| tor8 | sebras: sorry, cppcheck not coverity | 14:41.35 |
| sebras | (because I get no coverity e-mails) | 14:41.35 |
| tor8 | sebras: it's in the email to gs-regression | 14:41.42 |
| kens | Ah :-) | 14:41.44 |
| tor8 | New Ghostscript/GhostPDL compiler warnings - 2018-03-19-22:15:09 - 71bf7cb0c4aa3d2bb06deac2df675af44b3e4c3b | 14:41.47 |
| | regression | 14:41.48 |
| kens | You aren't signed up to Coverity sebras ? | 14:42.07 |
| tor8 | jbig2dec/jbig2_generic.c:760: error: Uninitialized variable: gbat | 14:42.12 |
| sebras | kens: not for my artifex account, no. | 14:43.46 |
| kens | Ah. | 14:43.52 |
| | I'm sure someone could add you if you want | 14:44.01 |
| sebras | kens: I'd probably need that to resolve this coverity warning. | 14:44.25 |
| kens | tor8 says cppcheck, not coverity | 14:44.35 |
| | But I'll see if I can add your Artifex account | 14:44.49 |
| sebras | tor8: was that "New Ghostscript/GhostPDL compiler warnings" a subject line in an e-mail? | 14:45.53 |
| kens | sebras well I can invite you, if you'll tell me your Artifex account :-) | 14:46.39 |
| sebras | kens: sebastian.rasmussen@artifex.com | 14:46.49 |
| kens | Right one sec | 14:46.55 |
| tor8 | sebras: yes. | 14:47.06 |
| sebras | tor8: can't find it. but I did find 'Regression 71bf7cb0c4aa3d2bb06deac2df675af44b3e4c3b (gs pcl xps) | 14:47.15 |
| | ' which didn't say anything about any warnings. | 14:47.25 |
| kens | sebras you should get an invitation email | 14:47.28 |
| sebras | or rather it says New warnings: None. | 14:47.36 |
| | kens: thanks kens! | 14:47.44 |
| kens | NP | 14:47.49 |
| tor8 | sebras: I forwarded it to you, so you can see what it looks like | 14:48.05 |
| sebras | tor8: if I search gs-cppcheck.txt I only get a match on your mail. | 14:50.07 |
| | tor8: not sure why I would be left out of that particular mail from gs-regression (I got the one whose subject I mentioned above) | 14:51.04 |
| mvrhel_laptop | hi tkamppeter | 15:10.42 |
| sebras | tor8: oh, cppcheck complains about gbat being uninitialized in the commit before mine, but at a different line, presumably this is why a regression was reported..? | 15:14.25 |
| tor8 | sebras: sounds plausible | 15:20.14 |
| ray_laptop | kens: chrisl: I am looking at bug 699129 where a command line parameter causes a spot color change. I've traced the failing case down to it having created a "Colorants" dictionary that has a count=3, maxlength=3, but the rsize of the keys and values arrays is 5, and when accessing using dict_index_entry, the three elements I expect are at 1, 2, 3 -- not 0, 1, 2. 0 gets an integer out of the... | 16:42.58 |
| | ...keys, not a name | 16:42.59 |
| kens | Not anythign I;'ve seen before | 16:44.01 |
| ray_laptop | have either of you delved into dictionaries (what's supposed to be where?) | 16:44.40 |
| kens | Presumably the thing to do is look at the creation of the object | 16:44.46 |
| ray_laptop | kens: I was afraid you'd say that. The scanner. YUCK | 16:45.20 |
| kens | sI've looked at it in the past, not recently | 16:45.25 |
| chrisl | ray_laptop: IIRC, teh rsize is the number of entries in the dictionary, *not* the number of *valid* entries in the dictionary | 16:46.12 |
| kens | That sounds likely, its possible to create a dicitonary or array with more slots than entities in it | 16:46.46 |
| chrisl | so, for example, "10 dict" will result in a dictionary with an r_size of 10, but it's emtpy | 16:46.48 |
| ray_laptop | note that I've added debug and dumping the Colorants dict with === looks OK. | 16:46.54 |
| | the pdict->maxlength is 3, in this case | 16:47.33 |
| chrisl | "maxlength - a t_integer whose value gives the client's view of the capacity (C). C may be less than M (see below)." | 16:49.35 |
| | idictdef.h | 16:49.46 |
| kens | So maxlength is the number of valid entries, rszie is the maximum (currently) number of entires. | 16:50.14 |
| | ? | 16:50.23 |
| chrisl | Basically, yes | 16:50.38 |
| ray_laptop | chrisl: where | 16:50.42 |
| | is that comment? | 16:50.49 |
| chrisl | idictdef.h | 16:50.56 |
| ray_laptop | nm | 16:50.58 |
| chrisl | For safety, you really ought to use the dictionary enumerator | 16:51.59 |
| ray_laptop | so, what I have here, in those terms is keys and values arrays M+2 elements | 16:52.09 |
| | chrisl: this is kens' code I think. zcolor.c devicencolorants_cont line 3721 | 16:53.44 |
| kens | I doubt its mine | 16:53.56 |
| | Hmm though a continuation procedure sounds reasonable | 16:54.13 |
| ray_laptop | I'll see if I can see what the scanner is doing when it creates the dict | 16:54.14 |
| kens | Are you saying the continuation procedure is causing the problem ? | 16:54.32 |
| ray_laptop | kens: yes, it works depending on the command line contents ;-/ | 16:54.37 |
| chrisl | Like I said, use the dictionary enumerator functions - that's how it's supposed to work | 16:54.56 |
| kens | If you can isolate it to that, reliably, then you can assign it to me | 16:55.10 |
| | But please write up how to reproduce it reliably | 16:55.29 |
| ray_laptop | kens: the call to dict_index_entry is calling with 0, 1, and 2 for a Colorants dict with 3 entries, but the keys that are t_name are at index 1, 2, 3 | 16:55.59 |
| kens | Could be this is soemthing that changed, but I wrote all that a *long* time ago | 16:56.19 |
| ray_laptop | kens: it's in the bug 699129 | 16:56.26 |
| kens | Does it include what you recently found ? I'm in the middle of somtehing else and I'd prefer not to have to come back and bug you about it in a day or two | 16:57.04 |
| ray_laptop | when it works, the key lookups for 0, 1, and 2 ARE names, the key lookup for 3 gives an integer. So the dict still has rsize of keys and values == 5, just entries are in different places | 17:00.20 |
| chrisl | The key/value pairs in dictionaries are effectively sparse arrays, so accessing directly by index is, in general, not going to work | 17:00.59 |
| kens | Is all this captured in the bug thread ? If so then just assign it to me | 17:01.04 |
| ray_laptop | kens: since I'm digging into it this far, I'll continue. If I find that the index is truly indeterminate, I'll suggest an alternative | 17:01.32 |
| kens | Its *bound* to be indeterminate, as chrisl just said. | 17:01.48 |
| | The order of evaluation of dictionaries is effectively random | 17:02.06 |
| | Changing the command line will tickle the memory layout, resulting in different ordering | 17:02.28 |
| chrisl | Which is why we have specific functions to enumerate the contents of a dictionary...... | 17:02.28 |
| ray_laptop | chrisl: well it doesn't match the doc in idictdef.h which says the arrays for keys and values are M+1, not M+2 | 17:02.51 |
| chrisl | ray_laptop: presumably that's for automatically created dictionaries - it can't know what size a Postscript program will ask for | 17:04.00 |
| ray_laptop | if the dict maxlength is 3, having to access the 4th element doesn't make sense to me | 17:04.06 |
| | chrisl: .dicttomark KNOWS what size to create | 17:04.34 |
| chrisl | But the dict operator doesn't | 17:04.52 |
| ray_laptop | chrisl: yes, the dict operator is TOLD what size to create | 17:05.41 |
| | in Level 2 it's not a hard limit | 17:06.00 |
| chrisl | So, what's your problem, then? | 17:06.16 |
| | If I do "10 dict" what size does Postscript think the dictionary is? | 17:06.59 |
| | I'd agree that "maxlength" isn't a good name for the value, though.... | 17:09.44 |
| ray_laptop | oh, I see something. dict_create_contents uses dict_round_size for the keys and values arrays. That's the culprit. So indexing over the dict_length is not guaranteed to work | 17:09.46 |
| chrisl | I think I said that, twice, some time ago | 17:10.14 |
| ray_laptop | chrisl: to answer your question if you do 10 dict dup length = maxlength = you get 0 and 10 | 17:11.12 |
| | chrisl: you did say that, but if you said "why" I missed it. | 17:11.39 |
| chrisl | I did say why - they are sparse arrays (on most platforms). That allows us to use hashing for a faster lookup | 17:12.31 |
| | I think if you build for a small memory architecture (which I doubt even works), they aren't sparse | 17:13.09 |
| | ray_laptop: anyway, as I said: we have (the badly named!) dict_first() and dict_next() specifically for enumerating throught the valid entries in a dictionary - that's what we ought to be using | 17:16.18 |
| ray_laptop | chrisl: I noticed that zfapi.c also uses dict_index_entry -- you (or kens) may want to look into that since it could trip over the same issue | 18:28.43 |
| | Forward 1 day (to 2018/03/22)>>> | |
Log of #ghostscript at irc.freenode.net.