Immutable not working correctly

[thomasms]

According to the issue in pvDatabase (see here) setImmutable does not actually work. It appears the issue lies in PVScalarValue::put.

Currently it is like below:

  /**
   * Put a new value into the PVScalar.
   * @param value The value.
   */
  inline void put(typename storage_t::arg_type v) {
    storage.store(v);
    PVField::postPut();
  }

This seems to fix the issue.

  /**
   * Put a new value into the PVScalar.
   * @param value The value.
   */
  inline void put(typename storage_t::arg_type v) {
    if (!isImmutable()) {
      storage.store(v);
    }
    PVField::postPut();
  }

I would suggest to throw an exception here if it is immutable, just letting it silently ignore the put operation is probably dangerous. I was thinking something like below.

    /**
     * Put a new value into the PVScalar.
     * @throws std::runtime_error if the field is immutable
     * @param value The value.
     */
    inline void put(typename storage_t::arg_type v) {
        if(isImmutable()){
            throw std::runtime_error("Cannot perform put operation - field is immutable");
        }
        storage.store(v);
        PVField::postPut();
    }

Would this be acceptable?

[mdavidsaver]

... setImmutable does not actually work ...

... I would suggest to throw an exception here if it is immutable ...

Would this be acceptable?

Yes.

Personally, I see isImmutable() as a poor solution to the two problems it seems intended to solve (remote ACL, and local const-like restrictions). I would recommend using explicit ACL checks at the boundary with server code to address the first case, and adding a healthy dose of const for the second (eg. working with shared_ptr<const PVField>).

Compare usage of PVField::setImmutable() with freeze() for a shared_vector. With freeze(), misuse results in an immediate compile error. With setImmutable(), issues won't be discovered until runtime.