On Fri, Mar 07, 2008 at 09:44:13PM +0200, Sanja Byelkin wrote:
> On Fri, Mar 07, 2008 at 10:29:25AM +0100, Guilhem Bichot wrote:
> [skip]
> > > @@ -3442,7 +3477,8 @@ my_bool translog_init_with_table(const c
> > > MYF(0));
> > > if (file == NULL ||
> > > (file->handler.file=
> > > - open_logfile_by_number_no_cache(i)) < 0)
> > > + open_logfile_by_number_no_cache(i)) < 0 ||
> > > + my_seek(file->handler.file, 0, SEEK_END, MYF(0)) >=
> LL(0xffffffff))
> >
> > In case of error during my_seek(), it will return MY_FILEPOS_ERROR.
> > my_off_t may be ulong: then when it is compared with the longlong
> > constant, my_off_t will be promoted to longlong, and then my_off_t
> > will show up as LL(-1), compared with LL(0xffffffff) the test above
> > could yield "false", i.e. we will not detect the seek error.
> > I suggest to catch the error explicitely: test if my_seek is ==
> > MY_FILEPOS_ERROR and then compare it with ULL(0xffffffff). Using ULL
> > guarantees that my_off_t will be cast to unsigned before comparison,
> > which is what we want (we don't want negative numbers in this
> > comparison, they would surely mean a problem).
> >
> > So, I suggest
> >
> > my_off_t file_size;
> >
> > ...
> >
> > if (file == NULL ||
> > (file->handler.file=
> > open_logfile_by_number_no_cache(i)) < 0)
> > open_logfile_by_number_no_cache(i)) < 0 ||
> > (file_size= my_seek(file->handler.file, 0, SEEK_END,
> > MYF(0)) == MY_FILEPOS_ERROR) ||
> > (file_size >= ULL(0xffffffff))
> > (not sure I wrote the () right but you get the idea).
> >
>
> But AFAIK MY_FILEPOS_ERROR just max possible value so >=
> ULL(0xffffffff) will test the error state also.
Ok, I don't think that in the future we will change MY_FILEPOS_ERROR
to something < ULL(0xffffffff), it sounds impossible.
So you can remove the test for ==MY_FILEPOS_ERROR and just use
my_seek >= ULL(0xffffffff)
