Is it should be safe or unsafe function?

Say, a function will exclusive access memory. However, it does not require the exclusive access in its body, instead, it assume the caller will do that. e. g.:

fn a() {
 ...
}

lock();
a();
unlock();

Then, should an unsafe be added to a? e.g.

unsafe fn a() {}

What do you mean "access memory"? What memory, where?

Typically one does not just access memory, one reads and writes values into variables. Which happen to occupy memory. Either located on the stack as local variables, or located in heap memory space, as things allocated with Box<> and the like.

Mostly one never needs unsafe to work with such variables.

Or, are you wanting ti read and write some arbitrary memory locations through a raw pointer? Then you will need unsafe.

It would help if you could elaborate your code example with something that actually accesses variables so that people know what you are asking about.

1 Like

What I am saying here is what is an unsafe function

Documents says:

Unsafe functions are functions that are not safe in all contexts and/or for all possible inputs. 

Then, a function requiring caller holds a lock is not safe in all contexts: it is safe only when caller holds a lock.

So, it should be an unsafe function, and I should define it as

unsafe fn a(){}

?

But, if it is, then:

refcell::borrow_mut() should be unsafe, either: it only safe when there is not another mut borrower.

Are you trying to mutate a value through an immutable &T reference? Your code doesn't say anything about what you are actually trying to do.

As for RefCell::borrow_mut: no, it doesn't need to be unsafe, because it enforces exclusivity at runtime. Just like Mutex::lock, etc.

In general, a function should be tagged unsafe if it relies on the caller doing anything not enforced by the type system (for safety purposes). Put another way, if it's possible at all for a malicious caller to make the function violate memory safety, the function needs to be marked unsafe.

This requires the unsafe keyword to appear at every callsite, which is a signal to code reviewers that there's some extra safety condition that compiler won't check, so they need to check for it manually.


No, borrow_mut is always safe to call. It checks the safety invariants at runtime and will panic! instead of allowing them to be violated. This generally signals a bug, but at least it doesn't leave the program running in a sort-of-working but unpredictable state: You get a clean error message that points directly to where (part of) the problem lies.

2 Likes

It really depends on what it does. Does it mutate a static mut global using the Mutex to ensure exclusive access?

No, it assumes the caller will hold the lock.
It is a very common case:

static void inline __list_add(list_head * new1,
                       list_head * prev,
                       list_head * next)
{
    next->prev = new1;
    new1->next = next;
    new1->prev = prev;
    prev->next = new1;
}

functions like above absolutely need a critical section to protect it.
But if implemented it as

static void inline __list_add(list_head * new1,
                       list_head * prev,
                       list_head * next)
{
    mutex.lock();
    next->prev = new1;
    new1->next = next;
    new1->prev = prev;
    prev->next = new1;
    mutex.unlock();
}

then every invoke of __list_add have to pay for the locking.
And, obviously, there indeed are contexts locking is unnecessary.

You're handwaving the details which would determine whether this function may be safe or not. This was written before the latest edit to OP's last post; some of the below may not apply.

In general you can't "just access memory": you need to have a handle (a pointer) to some particular value (a memory location) which you may read and/or write. Where does this pointer come from?

  • Is it passed in to a from the caller? Then you can make a take a &mut reference, which means the caller has to guarantee exclusive access. In this case a does not have to be an unsafe fn. But the caller may have to use unsafe to get the &mut reference before it calls a.

  • Is it the address of some static? Then I think you must be using static mut. Don't do that. But if you insist, you will be using unsafe inside a to access the shared memory, and you must make a an unsafe fn unless you can guarantee that the safety invariants required inside a are upheld by a itself. If one of those invariants is "such-and-such a lock must be held by the current thread", and you're relying on the caller to do that, then a must be an unsafe fn.

    Alternatively, you could make a accept some kind of token which indicates the lock is really, for-sure being held, and then a could be safe because it can only be called with the token that makes the access sound. This is basically what a MutexGuard is, only a MutexGuard holds the reference to the shared memory inside it so you can use it safely.

To be specific, which one of the following is correct?

This one:

    #[inline]
    unsafe fn __list_add(new: *mut ListHead, prev: *mut ListHead, next: *mut ListHead) {
        (*next).prev = new;
        (*new).next = next;
        (*new).prev = prev;
        (*prev).next = new;
    }

    #[inline]
    pub unsafe fn list_add(&mut self, new: &mut ListHead) {
        ListHead::__list_add(new, self, (*self).next);
    }

or this one:

    #[inline]
    unsafe fn __list_add(new: *mut ListHead, prev: *mut ListHead, next: *mut ListHead) {
        (*next).prev = new;
        (*new).next = next;
        (*new).prev = prev;
        (*prev).next = new;
    }

    #[inline]
    pub fn list_add(&mut self, new: &mut ListHead) {
        unsafe {
            ListHead::__list_add(new, self, (*self).next);
        }
    }

It does be &mut in caller side, but &mut is not enough to ensure the safety.
The focus is: then list_add should be tagged as unsafe?

Maybe neither. Doubly linked lists are inherently share-y; it's tricky to make one safe especially if you ever promote a pointer to &mut. I'm not sure if list_add can necessarily be called at all in some situations -- for instance, what is the value of (*self).next when the list is empty? Bear in mind that unsafe doesn't actually relax any of the rules around references and safe access to memory: it just takes down the compiler guardrails that normally tell you when you're getting too close.

I beg to differ: linked lists are pretty uncommon in Rust, partly because they have ambiguous ownership semantics but partly because they're just not that great in general, and the reasons they're more common in C don't really apply to Rust. In particular, Rust has generics and sum types (i.e., enum) which largely eliminate the need for type erasure in data structures.

If you're trying to make a Rust API to a C library that contains a linked list, it may just have to be unsafe, because everything in C is unsafe, and calling C from Rust is likewise unsafe. But correctness depends on the whole API and not just on fragments of it. If you're trying to make a linked list in Rust, well... read the book first: Introduction - Learning Rust With Entirely Too Many Linked Lists

1 Like

If every function that called an unsafe function itself had to be marked unsafe then it would be impossible to ever have any function that is not marked as unsafe.

After all, at the bottom of the rabbit hole are functions that are doing I/O which has to contain unsafe. The unsafe would propagate upwards to all other functions one could ever write.

I conclude that somewhere along the call chain there has to be a function that contains an unsafe block and is not itself marked unsafe.

// Must be called from unsafe block
unsafe fn uf() {
}

// The unsafe stops here.
fn f () {
    //...

    // Contains a bit of unsafe
    unsafe {
        uf();
    }

    // ...
}

// All safe code...
fn main() {
    f();
}

I'm not sure what the talk of locks was about. Surely they are only needed when you have threads involved.

Building a linked list in Rust is not so easy. Liked lists are almost never needed or a good idea.

1 Like

The list_add function can't be correctly used. It takes mutable references. These are invalidated once you return from list_add, which means that once list_add returns the ListHead's now contain pointers that can't be used to access anything. You will have to use raw pointers all the way up. Wrapping it in a struct like struct ListPtr<'a>(*mut ListHead, PhantomData<&'a mut ListHead>); would allow recovering some safety though.

1 Like

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.