From: Michal H. <ms...@gm...> - 2010-04-28 11:34:07
|
On Tue, Apr 27, 2010 at 11:19:32PM +0200, Michal Hocko wrote: > On Tue, Apr 27, 2010 at 08:07:35PM +0300, Jozef Misutka wrote: > > On Tue, 27 Apr 2010 15:52:35 +0300, Michal Hocko <ms...@gm...> wrote: > > > > >On Tue, Apr 27, 2010 at 01:37:13PM +0300, Jozef Misutka wrote: > > >>On Mon, 26 Apr 2010 20:29:05 +0300, Michal Hocko > > >><ms...@gm...> wrote: > > >> > > >>please michal, fix this issue as I asked you yesterday. thank you > > >> > > >>>> > > >>>>5>..\..\src\xpdf\xpdf\XRef.cc(815) : error C3861: 'atoll': > > >>>>identifier not found > > >>>>5>..\..\src\xpdf\xpdf\XRef.cc(825) : error C3861: 'atoll': > > >>>>identifier not found > > > > > ><muttering> > > >Well, you have ACKed the patch so I assumed you have at least looked at > > >it or tried to compile it. Now you are asking me to fix it even though > > >it doesn't work only on Windows (the function is POSIX). > > ></muttering> > > > > Does anyone in your company recompile everything you commit? i would > > spend far more less time doing it by myself.... > > I was just picking at you. But, really, review of a such a simple patch > isn't that much of work (just a proper ret. value checks + proper casting) > and you know that those functions can be tricky for portability outside > POSIX (e.g. atoi and atol are POSIX and C99 but I haven't noticed that > atoll is POSIX only) and I have no way how to test it. So I assumed that > you would compile test it as well. > > My primary point was that we have certain problems with changes > reviewing. I remember several patches (mostly mine) were you gave an ACK > and then - after patch was long in - you have found out a compilation > issue on Windows which never popped out in my local builds. > > Anyway, this was really my fault in the first place - I will try to > check CONFORMING TO section of man pages more carefully. > > > > > > > >Check the attached patch. > > And it seems you have ignored it and reverted the change with log > message which makes quite a good sense: > " > [...] > 2. atoll converted to atol > [...] > " Btw. the change is not correct as well: @@ -812,7 +813,7 @@ GBool XRef::constructXRef() { // look for object } else if (isdigit((unsigned char)(*p))) { - num = (int)atoll(p); + num = (int)atol(p); if (num > 0) { do { ++p; @@ -822,7 +823,7 @@ GBool XRef::constructXRef() { ++p; } while (*p && isspace(*p)); if (isdigit((unsigned char)*p)) { - gen = (int)atoll(p); + gen = (int)atol(p); do { ++p; } while (*p && isdigit((unsigned char)*p)); This doesn't work on platforms where sizeof(int) == sizeof(long) == 32 > > Which we will understand 2 years later when we see a similar issue... > > Should I refresh the patch on top of your revert or we simply ended up > ignoring those objects with gen larger than 2B? > > > -- > Michal Hocko > > ------------------------------------------------------------------------------ > _______________________________________________ > Pdfedit-devel mailing list > Pdf...@li... > https://lists.sourceforge.net/lists/listinfo/pdfedit-devel -- Michal Hocko |