On the topic of 'Creates a temporary, which is freed etc.', again

1

I am simply trying to read the entries of a directory (Ubuntu Linux 22.10, ext4 filesystem):

    let mut all_chosenfiles: Vec<&str> = Vec::new();
    for next_path in path_components {
        for next_dir_entry in fs::read_dir(next_path) {
            for next_file in next_dir_entry {
                let file_metadata = next_file.as_ref().unwrap().metadata().unwrap();
                if file_metadata.is_file()
                    && file_metadata.mode() & 0o500 == 0o500 {
                        let chosen_file = next_file.unwrap().path();
                        all_chosenfiles.push(chosen_file.to_owned().to_str().unwrap());
                }

            }
        }
    }

Not very idiomatic, I agree, but it serves the purpose at hand.

The compiler is not happy though:

error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:31:46
   |
31 |                         all_chosenfiles.push(chosen_file.to_owned().to_str().unwrap());
   |                         ---------------------^^^^^^^^^^^^^^^^^^^^^^-------------------- temporary value is freed at the end of this statement
   |                         |                    |
   |                         |                    creates a temporary which is freed while still in use
   |                         borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

I understand what is she complaining about. I have tried to pacify her, by using let as has been prescribed [here]( [1]: E0716 - Error codes index):

    for next_path in path_components {
        for next_dir_entry in fs::read_dir(next_path) {
            for next_file in next_dir_entry {
                let file_metadata = next_file.as_ref().unwrap().metadata().unwrap();
                if file_metadata.is_file()
                    && file_metadata.mode() & 0o500 == 0o500 {
                        let chosen_path = next_file.unwrap().path();
                        let chosen_file = &chosen_path.to_owned().to_str().unwrap();
                        all_chosenfiles.push(chosen_file);
                }

            }
        }
    }

That `to_owned()' should have worked, IMO, but she is stubborn:

error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:31:44
   |
31 |                         let chosen_file = &chosen_path.to_owned().to_str().unwrap();
   |                                            ^^^^^^^^^^^^^^^^^^^^^^                  - temporary value is freed at the end of this statement
   |                                            |
   |                                            creates a temporary which is freed while still in use
32 |                         all_chosenfiles.push(chosen_file);
   |                                              ----------- borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

I am scratching my head. I am missing some idiom, I am sure.

Is a derefencing using '*' operator (viz., *chosent_file) the only way out?

I have gotten around, for the time being, by:

let mut all_chosenfiles: Vec<String> = Vec::new(); // of String, and not of &str
    for next_path in path_components {
        for next_dir_entry in fs::read_dir(next_path) {
            for next_file in next_dir_entry {

                let file_metadata = next_file.as_ref().unwrap().metadata().unwrap();
                if file_metadata.is_file()
                    && file_metadata.mode() & 0o500 == 0o500 {
                        // Using as_ref() before unwrapping
                        let chosen_path: PathBuf = next_file.as_ref().unwrap().path();
                        // Doc says that, to_str()
                        // "Yields a &str slice if the Path is valid unicode"
                        let chosen_file: Option<&str> = chosen_path.to_str();
                        // Explicitly owning a String, made out of &str, below!
                        all_chosenfiles.push(chosen_file.unwrap().to_owned());
                }

            }
        }
    }

Honestly, I am not really sure why it is working. By calling next_file.as_ref(), I ensure that a reference to that memory is held. But, how does that solve the problem of temporary which is freed etc. issue? Moreover, cloning the pathstrings by using to_str() seems to be wasteful to me.

There must be an idiom or a compact set of rules to follow, whenever one wants to write expressions like these.

Could you please point out, the gaps in my understanding?

2

The first uses &str, which is a reference type, and you may not have a reference to a value past its scope (a temporary is a value whose scope ends at the expression where its created).

The 2nd example uses String, which owns its memory, so there is no issue with keeping Strings "arbitrarily" long past the point where they were created.

to_str doesn't appear to clone the PathBuf hete (but I may be wrong)

3

You can't use Vec<&str> for this. This type means "this Vec contains no strings in it, they all have already been stored elsewhere", and you have not stored them elsewhere (to_owned() creates a temporary value that is destroyed before code reaches the next line, and you're not storing it, because to_str() only borrows it).

A Vec that actually contains its own strings is a Vec<String>.

For file system paths, you should use Vec<PathBuf>, which works with non-Unicode file systems too. Rust strings are always Unicode and to_str().unwrap() will blow up on file names that aren't ASCII or UTF-8.

4

Here's how I might rewrite this.

fn foo<I, P>(path_components: I) -> io::Result<Vec<PathBuf>>
where
    I: IntoIterator<Item = P>,
    P: AsRef<Path>,
{
    let mut all_chosenfiles: Vec<PathBuf> = Vec::new();
    for next_path in path_components {
        for dir_entry in fs::read_dir(next_path)? {
            let next_file = dir_entry?;
            let metadata = next_file.metadata()?;
            if metadata.is_file() && metadata.mode() & 0o500 == 0o500 {
                all_chosenfiles.push(next_file.path());
            }
        }
    }
    Ok(all_chosenfiles)
}

It will fail eagerly, so if the caller still wants to unwrap and panic, they can.

  • Factoring things out into a Result-returning function allows for ergonomic use of ? to bail, instead of
    • Using as_ref to not comsume transitive operations on the Results
    • Lots of unwrap
    • iterating over Results
  • It avoids making something owned (next_file.path() is a PathBuf), borrowing it, then making something else owned from the borrow. That's effectively a (fallible) clone.
  • It can handle many input types (I wasn't sure what your path_components was)

If you must convert to String for some reason... well, hopefully you don't, that's not very portable. But if you do, I'll grant the maneuvering between PathBuf and OsString and String, and displaying OsStrings, has much room for improvement. But you can still make it Result friendly with something like this helper function:

fn path_buf_as_string(path: PathBuf) -> io::Result<String> {
    let as_ref = <OsStr as AsRef<Path>>::as_ref;
    path.into_os_string()
        .into_string()
        .map_err(|e| {
            io::Error::new(
                io::ErrorKind::InvalidData,
                format!("non-UTF8 filename {}", as_ref(&e).display())
            )
        })
}

-    let mut all_chosenfiles: Vec<PathBuf> = Vec::new();
+    let mut all_chosenfiles: Vec<String> = Vec::new();
     for next_path in path_components {
         for dir_entry in fs::read_dir(next_path)? {
             let next_file = dir_entry?;
             let metadata = next_file.metadata()?;
             if metadata.is_file() && metadata.mode() & 0o500 == 0o500 {
-                all_chosenfiles.push(next_file.path());
+                let next_file = path_buf_as_string(next_file.path())?;
+                all_chosenfiles.push(next_file);
             }
5

Regarding the error message and your intuition to use a let, I think you did not understand the error message correctly, because the error message is fairly hard to understand: The “borrow later used here” effectively refers to the fact that all_chosenfiles is accessed in any way after the current statement, in this care even after the current loop iteration, yet in this case the later-used-here point syntactically appears within the same statement. Effectively “later” in the error is referring to the next .push call one loop iteration later. That's particularly hard to understand not only since the two things are part of the same expression, but also because the connection between the scope of chosen_file.to_owned()'s temporary and the lifetime in the Vec<&str> type of all_chosenfiles is left to the reader to figure out. I. e. you need to come up yourself with the explanation for: why does calling a method on all_chosenfiles constitute a usage of the borrow of the temporary variable holding chosen_file.to_owned()? The answer to this question is:

  • the to_str call implicitly borrows the temporary in question, and to_str is (&'a self) -> &'a str, so the returned string slice reference is considered to borrow from this temporary, because they use the same lifetime in the type signature
  • the type signature of push is fn(&mut Vec<T>, T), and we push a &str, so it is fn(&mut Vec<&'a str>, &'a str), which - again - due to the two lifetimes being the same - had the effect that after the push call the vector, too is considered to be a borrow of the temporary in question

to re-iterate the rest of the process: after the push call in one loop iteration connected the lifetimes and made the vector into a value borrowing from the temporary in question, the statement ends and the temporary is dropped, and then the next push call one iteration later accesses the vector again. Accessing a value that borrowed from a dropped value is forbidden, hence the error.

2 Likes
6

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.