From 45b508dff6d437368156cec30aae394bcf9edf7d Mon Sep 17 00:00:00 2001
From: Scott Lamb <slamb@slamb.org>
Date: Fri, 22 Sep 2017 06:47:08 -0700
Subject: [PATCH] fix moonfire_ffmpeg::Error formatting

This would return a string with trailing nuls up to the buffer size (64
bytes) which would cause problems later. One in particular: in
"moonfire-nvr config", if testing a camera failed, displaying the error would
panic with the backtrace below.

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: NulError(33, [69, 114, 114, 111, 114, 58, 32, 102, 102, 109, 112, 101, 103, 58, 32, 67, 111, 110, 110, 101, 99, 116, 105, 111, 110, 32, 114, 101, 102, 117, 115, 101, 100, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0])', src/libcore/result.rs:860:4
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::begin_panic_new
   5: std::panicking::begin_panic_fmt
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::result::unwrap_failed
   9: <core::result::Result<T, E>>::unwrap
  10: <&'a str as ncurses::ToCStr>::to_c_str
  11: ncurses::addstr
  12: ncurses::mvaddstr
  13: <cursive::backend::curses::n::Concrete as cursive::backend::Backend>::print_at
  14: cursive::printer::Printer::print
  15: <cursive::views::text_view::TextView as cursive::view::View>::draw::{{closure}}
  16: cursive::view::scroll::ScrollBase::draw
  17: <cursive::views::text_view::TextView as cursive::view::View>::draw
  18: <cursive::views::dialog::Dialog as cursive::view::View>::draw
  19: <cursive::views::layer::Layer<T> as cursive::view::view_wrapper::ViewWrapper>::wrap_draw
  20: cursive::view::view_wrapper::<impl cursive::view::View for T>::draw
  21: <cursive::views::shadow_view::ShadowView<T> as cursive::view::view_wrapper::ViewWrapper>::wrap_draw
  22: cursive::view::view_wrapper::<impl cursive::view::View for T>::draw
  23: <cursive::views::stack_view::StackView as cursive::view::View>::draw::{{closure}}
  24: cursive::printer::Printer::with_color::{{closure}}
  25: <cursive::backend::curses::n::Concrete as cursive::backend::Backend>::with_color
  26: cursive::printer::Printer::with_color
  27: <cursive::views::stack_view::StackView as cursive::view::View>::draw
  28: cursive::Cursive::draw
  29: cursive::Cursive::step
  30: cursive::Cursive::run
  31: moonfire_nvr::cmds::config::run
  32: moonfire_nvr::cmds::Command::run
  33: moonfire_nvr::main
  34: __rust_maybe_catch_panic
  35: std::rt::lang_start
  36: main
---
 ffmpeg/lib.rs | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/ffmpeg/lib.rs b/ffmpeg/lib.rs
index 9273868..3fb1926 100644
--- a/ffmpeg/lib.rs
+++ b/ffmpeg/lib.rs
@@ -76,7 +76,7 @@ extern "C" {
 //#[link(name = "avutil")]
 extern "C" {
     fn avutil_version() -> libc::c_int;
-    fn av_strerror(e: libc::c_int, b: *mut u8, s: libc::size_t) -> libc::c_int;
+    fn av_strerror(e: libc::c_int, b: *mut libc::c_char, s: libc::size_t) -> libc::c_int;
     fn av_dict_count(d: *const AVDictionary) -> libc::c_int;
     fn av_dict_get(d: *const AVDictionary, key: *const libc::c_char, prev: *mut AVDictionaryEntry,
                    flags: libc::c_int) -> *mut AVDictionaryEntry;
@@ -356,9 +356,13 @@ impl std::error::Error for Error {
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         const ARRAYLEN: usize = 64;
-        let mut buf = [0u8; ARRAYLEN];
-        unsafe { av_strerror(self.0, buf.as_mut_ptr(), ARRAYLEN) };
-        f.write_str(std::str::from_utf8(&buf).map_err(|_| fmt::Error)?)
+        let mut buf = [0; ARRAYLEN];
+        let s = unsafe {
+            // Note av_strerror uses strlcpy, so it guarantees a trailing NUL byte.
+            av_strerror(self.0, buf.as_mut_ptr(), ARRAYLEN);
+            CStr::from_ptr(buf.as_ptr())
+        };
+        f.write_str(&s.to_string_lossy())
     }
 }
 
@@ -431,3 +435,19 @@ impl Ffmpeg {
         Ffmpeg{}
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::ffi::CString;
+    use super::Error;
+
+    #[test]
+    fn test_error() {
+        let eof_formatted = format!("{}", Error::eof());
+        assert!(eof_formatted.contains("End of file"), "eof is: {}", eof_formatted);
+
+        // Errors should be round trippable to a CString. (This will fail if they contain NUL
+        // bytes.)
+        CString::new(eof_formatted).unwrap();
+    }
+}