New submission from Eric Kow <[email protected]>: Is there anybody who can spare a moment to look at what I've found in my quick IO audit?
Do we need to set more handles to binary mode here? 1 patch for repository [email protected]:darcs/releases/branch-2.4: Sat May 15 11:39:48 BST 2010 Eric Kow <[email protected]> * DRAFT: comments on hPut and hGet in source code This is NOT meant for the darcs repo. Context is that GHC 6.12's locale-sensitive IO broke our SSH stuff. This is a good chance to make sure we go through our IO a bit more thorogughly. I did a grep -r src hPut and grep -r src hGet in my text editor and stepped through each instance. Here I've flagged the ones that gave me pause. ---------- files: draft_-comments-on-hput-and-hget-in-source-code.dpatch, unnamed messages: 11072 nosy: kowey status: needs-review title: DRAFT: comments on hPut and hGet in source code __________________________________ Darcs bug tracker <[email protected]> <http://bugs.darcs.net/patch249> __________________________________
New patches: [DRAFT: comments on hPut and hGet in source code Eric Kow <[email protected]>**20100515103948 Ignore-this: 44bffb9874e0826bcef549e180dd992a This is NOT meant for the darcs repo. Context is that GHC 6.12's locale-sensitive IO broke our SSH stuff. This is a good chance to make sure we go through our IO a bit more thorogughly. I did a grep -r src hPut and grep -r src hGet in my text editor and stepped through each instance. Here I've flagged the ones that gave me pause. ] hunk ./src/Darcs/Commands/Record.lhs 277 PriorPatchName p -> return p NoPatchName -> prompt_patchname False putStrLn "What is the log?" + -- also ok because we expect this smart locale stuff to work thelog <- lines `fmap` Ratified.hGetContents stdin return (p, thelog, Nothing) gl (LogFile f:fs) = hunk ./src/Darcs/Commands/Record.lhs 345 Just f) 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 f $ \h -> do case fc of _ | null (lines fc) -> hPutStrLn h oldname 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? mvare <- newEmptyMVar forkIO ((Ratified.hGetContents e >>= -- ratify: immediately consumed hPutStr stderr) 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 where putHeader field value = B.hPut h (B.append (formatHeader field value) newline) hunk ./src/Darcs/External.hs 398 withOpenTemp $ \(hat,at) -> do ftable' <- case mbundle of Just (content,bundle) -> do - hPutDocLn hat $ bundle + hPutDocLn hat $ bundle -- ratify hPutDocLn (withOpenTemp is bin mode) return [ ('b', renderString content) , ('a', at) ] Nothing -> return [ ('b', renderString body) ] hunk ./src/Darcs/External.hs 415 withCString cc $ \ccp -> withCString s $ \sp -> withOpenTemp $ \(h,fn) -> do - hPutDoc h body + hPutDoc h body -- ratify hPutDocLn (withOpenTemp is bin mode) hClose h writeDocBinFile "mailed_patch" body cfn <- canonFilename fn hunk ./src/Darcs/External.hs 440 if use_sendmail || scmd /= "" then do withOpenTemp $ \(h,fn) -> do + -- ratify hPutStrLn (withOpenTemp is bin mode) hPutStrLn h $ "To: "++ t hPutStrLn h $ find_from (linesPS body) hPutStrLn h $ find_subject (linesPS body) 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 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 hunk ./src/Darcs/External.hs 529 + -- AUDIT: what about here? hPutStr stderr) `finally` putMVar mvare ()) out <- B.hGetContents o hunk ./src/Darcs/External.hs 670 pipeDocToPager c args pr inp = withoutNonBlock $ withoutProgress $ do tmp <- tempdir_loc bracket (openBinaryTempFile tmp "darcs-pager") cleanup $ \(fn,fh) -> - do hPutDocWith pr fh inp + do hPutDocWith pr fh inp -- ratify hPutDocWith (openBinaryTempFile) hClose fh bracket (openBinaryFile fn ReadMode) hClose $ \h -> do x <- do pid <- runProcess c args Nothing Nothing (Just h) Nothing Nothing hunk ./src/Printer.lhs 146 -- | @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 Context: [News entries for Darcs 2.4.4. Eric Kow <[email protected]>**20100515083529 Ignore-this: 8f22a502764329c80c9d0bf24b4c7f96 ] [Resolve issue1841: Apply binary mode to ssh process and patch file handles. Jeremy Cowgar <[email protected]>**20100512194935 Ignore-this: 8da8ddae9e95ba474c43b9d99430efd3 ] [Remove more not + redirections usage in testsuite (breaks win32). Petr Rockai <[email protected]>**20100511213008 Ignore-this: 6f6ff7795fc1916934a91949ff9217f4 ] [Do not redirect output to a file when using "not" to avoid trouble on win32. Petr Rockai <[email protected]>**20100511211032 Ignore-this: 8f28cf755ece79f92de5e7ea4707170d ] [Avoid --debug in issue381.sh which currently breaks on GHC 6.12/win32. Petr Rockai <[email protected]>**20100511185534 Ignore-this: 9c7ec2fa83bd76412638503c74943cf0 ] [Use the correct FileOffset import on all GHCes on win32. Petr Rockai <[email protected]>**20100511182621 Ignore-this: a95f1421cbc0602cd4d777c9ff0ffcb5 ] [TAG 2.4.3 Eric Kow <[email protected]>**20100509132315 Ignore-this: 1a920525d0cd01c352d5a2893bbea10d ] Patch bundle hash: c835a7530920552bc910eb87d649de304712f3e8
unnamed
Description: Binary data
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
