On 01/03/2018 03:37 AM, Janne Blomqvist wrote: > On Sat, Dec 30, 2017 at 10:58 PM, Jerry DeLisle <jvdeli...@charter.net> wrote: >> On 12/30/2017 12:35 PM, Janne Blomqvist wrote: >>> On Sat, Dec 30, 2017 at 7:16 PM, Thomas Koenig <tkoe...@netcologne.de> >>> wrote: >> ---snip--- >>> >>> I can provide that stuff as a separate patch, or merge it into the >>> original megapatch and resubmit that, whichever way you prefer. >> >> I would prefer we split into two patches. This will make review of the >> library >> I/O changes easier. The int len is used in a lot of places also where it >> really >> happens to also be the kind (which is a length in our implementation). > > Hi, > > attached is a patch that makes the two attached testcases work. It > applies on top of the charlen->size_t patch. In the formatted I/O > stuff, I have mostly used ptrdiff_t to avoid having to deal with > signed/unsigned issues, as the previous code was using int. > > >
Well I have started reviewing. One concern I have is that in many places you are changing formatted field widths, like w in read_l, to ptrdiff_t. I don't think this is necessary. If someone is printing a value that has a field width that big, it will never finish. I did not check the definitions of format data structures that use these values all over. So I think in the least, the patch goes too far. Another issue I have is that the readability of the code just sucks to me. The type ptrdiff_t is so not intuitive in a very many places. If this is really necessary lets hide it behind a macro or something so I don't have to cringe every time I look at this code. (such as gfc_int) Sometimes we can get too pedantic about things like this. A lot of these variables have nothing to do with pointers, though they may be used as modifiers to offsets in large memory spaces. I need a day or two to go through all of this. Sorry if I am just a downer about it. Regards, Jerry
