Yeah, I think you want something like this: git checkout master # start with master git checkout -b test # make a new branch git pull https://github.com/lgritz/oiio.git lg-tga # pull my changes
On Oct 30, 2012, at 5:17 PM, Nicolas Burtnyk wrote: > Excellent! I can give it a spin. Off-topic Git n00b question: is > there an easy was to grab just the changes in that pull request into > my fork of oiio? > > > On Tue, Oct 30, 2012 at 5:15 PM, Larry Gritz <[email protected]> wrote: >> Pull request updated. I think there were a lot of little bugs in the >> writing of the extensions area in the file. >> I believe I've got it all fixed now. >> >> >> On Oct 30, 2012, at 4:17 PM, Larry Gritz wrote: >> >>> Yeah, there are a few more things that look very sketchy. Let me take >>> another look... >>> >>> >>> >>> On Oct 30, 2012, at 3:04 PM, Nicolas Burtnyk wrote: >>> >>>> Hi Larry, >>>> >>>> I took a peek at your pull request and although it looks like this >>>> change will cleanly close tga files on write errors, it still seems >>>> like we're bailing too early in the case of empty author or document >>>> name fields. >>>> >>>> For example, in the block of code below, if the image spec doesn't >>>> have an "Artist" attribute (and hence tmpstr is empty), we early >>>> return false form write_tga20_data_fields skipping all other fields. >>>> >>>> if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) >>>> return false; >>>> >>>> Is that the intended behavior? >>>> >>>> I guess the core of the problem is that the fwrite helper function is >>>> returning false not only on write errors, but also when it is asked to >>>> write zero bytes and no attempt is made to distinguish bewteen those 2 >>>> cases. >>>> >>>> Perhaps the solution is to have the helper fwrite return true when >>>> itemsize or nitems is zero? >>>> >>>> Also, I noticed the block of code that fills with zeros after writing >>>> the "job ID" field (taken from the "DocumentName" attribute) looks >>>> kind of sketchy: >>>> >>>> // job ID >>>> tmpstr = m_spec.get_string_attribute ("DocumentName", ""); >>>> if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) >>>> return false; >>>> >>>> // fill the rest with zeros >>>> for (int i = 41 - std::min (tmpstr.length (), size_t(40)); i > 0; i--) { >>>> if (!fwrite (&tmpint) || >>>> !fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) >>>> return false; >>>> } >>>> >>>> Why is it trying to write the DocumentName string again every time it >>>> successfully writes 1 zero? >>>> >>>> >>>> -Nicolas >>>> >>>> >>>> On Tue, Oct 30, 2012 at 12:44 PM, Larry Gritz <[email protected]> wrote: >>>>> https://github.com/OpenImageIO/oiio/pull/448 >>>>> >>>>> >>>>> On Oct 30, 2012, at 10:53 AM, Larry Gritz wrote: >>>>> >>>>>> Thanks for pointing that out. >>>>>> >>>>>> Yes, in fact there are also several places in TGAOutput::open() where >>>>>> the file handle is not properly closed. >>>>>> >>>>>> Sounds easy to fix. Stay tuned for a pull request from me. >>>>>> >>>>>> >>>>>> >>>>>> On Oct 30, 2012, at 10:39 AM, Nicolas Burtnyk wrote: >>>>>> >>>>>>> While testing OIIO, I noticed that TGAs that were being written to >>>>>>> disk were not being closed despite calling ImageOutput::close(). >>>>>>> Indeed fclose is not called in TGAOutput::close when the "Artist" >>>>>>> attribute is empty. >>>>>>> The problematic block of code is: >>>>>>> >>>>>>> if (!fwrite (tmpstr.c_str(), std::min (tmpstr.length (), size_t(40)))) { >>>>>>> return false; >>>>>>> } >>>>>>> >>>>>>> Since tmpstr is empty, the itemsize parameter of fwrite is 0 and >>>>>>> fwrite returns false, which in turn makes TGAOutput::close early >>>>>>> return false (skipping the call to fclose). >>>>>>> >>>>>>> Writing of other string attributes are similarly problematic when the >>>>>>> attribute is empty. >>>>>>> >>>>>>> -Nicolas >>>>> >>>>> -- >>>>> Larry Gritz >>>>> [email protected] >>>>> >>>>> >>>>> _______________________________________________ >>>>> Oiio-dev mailing list >>>>> [email protected] >>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>>> _______________________________________________ >>>> Oiio-dev mailing list >>>> [email protected] >>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >>> >>> -- >>> Larry Gritz >>> [email protected] >>> >>> >>> _______________________________________________ >>> Oiio-dev mailing list >>> [email protected] >>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org >> >> -- >> Larry Gritz >> [email protected] >> >> >> _______________________________________________ >> Oiio-dev mailing list >> [email protected] >> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org > _______________________________________________ > Oiio-dev mailing list > [email protected] > http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org -- Larry Gritz [email protected] _______________________________________________ Oiio-dev mailing list [email protected] http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
