Hmm, I should actually have sent that to darcs-users instead of the patch tracker
Following up on myself, looking more carefully at this, I get the impression that nothing here is release-critical. So if nobody says anything, I'll put 2.4.4 online for packagers by tomorrow. Let this one be right this time... On Sat, May 15, 2010 at 10:44:51 +0000, Eric Kow wrote: > Is there anybody who can spare a moment to look at what I've found in my > quick IO audit? I wonder if there's something we need to be doing in the bigger picture so that these sorts of audits aren't needed. Seems like we have a hard time knowing what our IO is doing :-( > append_info f oldname = > do fc <- readBinFile f > + -- AUDIT: seems incongruous that that we're reading bin file > + -- and writing back out in text mode appendToFile uses bin mode underneath, so that's OK > hunk ./src/Darcs/External.hs 321 > (i,o,e,pid) <- runInteractiveProcess c args Nothing Nothing > hSetBinaryMode i True > hSetBinaryMode o True > + -- AUDIT: should e be also in binary mode (probably doesn't matter) > + -- what about stdout and stderr? This is in pipeDoc... and the answer doesn't really seem to affect correctness (just user output) > hunk ./src/Darcs/External.hs 364 > putHeader "Subject" s > when (not (null cc)) (putHeader "Cc" cc) > putHeader "X-Mail-Originator" "Darcs Version Control System" > + -- AUDIT: should this be binary mode? what's the body? > + -- if the body is string-based it will use hPutStrLn underneath > + -- if the body is PS-based, it will use hPut > hPutDocLn h body This is generateEmail. All the uses I see for generate email open the handle in bin mode. Maybe that's something we should haddock? 'generateEmail expects handles open in bin mode' > hunk ./src/Darcs/External.hs 505 > execDocPipe :: String -> [String] -> Doc -> IO Doc > execDocPipe c args instr = withoutProgress $ > do (i,o,e,pid) <- runInteractiveProcess c args Nothing Nothing > + -- AUDIT: seems like we should also set ioe binary mode here > + -- in case we have string-based docs underneath > forkIO $ hPutDoc i instr >> hClose i > mvare <- newEmptyMVar > forkIO ((Ratified.hGetContents e >>= -- ratify: immediately consumed This is used for signPGP stuff > hunk ./src/Darcs/External.hs 525 > execPipeIgnoreError :: String -> [String] -> Doc -> IO Doc > execPipeIgnoreError c args instr = withoutProgress $ > do (i,o,e,pid) <- runInteractiveProcess c args Nothing Nothing > + -- AUDIT: seems like we should also set ioe binary mode here > forkIO $ hPutDoc i instr >> hClose i > mvare <- newEmptyMVar > forkIO ((Ratified.hGetContents e >>= -- ratify: immediately consumed This is used for talking to external diff tools by the Darcs diff command. > -- | @hPrintPrintable h@ prints a 'Printable' to the handle h. > hPrintPrintable :: Handle -> Printable -> IO () > -hPrintPrintable h (S ps) = hPutStr h ps > +hPrintPrintable h (S ps) = hPutStr h ps -- AUDIT: is this really the right > thing? > hPrintPrintable h (PS ps) = B.hPut h ps > hPrintPrintable h (Both _ ps) = B.hPut h ps Seems like it's your business how you open the handle here -- Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow> PGP Key ID: 08AC04F9
pgpSlzqCRjRHL.pgp
Description: PGP signature
_______________________________________________ darcs-users mailing list darcs-users@darcs.net http://lists.osuosl.org/mailman/listinfo/darcs-users
