archlinux / alpm.rs

Rust bindings for libalpm
GNU General Public License v3.0
112 stars 21 forks source link

Non-optional `Package::arch` ? #15

Closed fosskers closed 3 years ago

fosskers commented 3 years ago

I noticed that Package::arch returns in Option:

    pub fn arch(&self) -> Option<&'a str> {
        let arch = unsafe { alpm_pkg_get_arch(self.pkg) };
        unsafe { from_cstr_optional(arch) }
    }

Yet alpm itself seems total:

/** Returns the architecture for which the package was built.
 * @param pkg a pointer to package
 * @return a reference to an internal string
 */
const char *alpm_pkg_get_arch(alpm_pkg_t *pkg);

which has the exact same structure as name:

const char *alpm_pkg_get_name(alpm_pkg_t *pkg);

which is a total function in the Rust. Could the Rust also return a plain &'a str for arch()?

Morganamilo commented 3 years ago

Only name and version are actually required. Everything may not actually exist. Docs will get there eventually.

fosskers commented 3 years ago

When it doesn't actually exist, what happens when alpm_pkg_get_arch is called? Do you get a null pointer?

Morganamilo commented 3 years ago

Yep.

Morganamilo commented 3 years ago

If you do plan to fix the docs. For the love of god pleas hold of until my huge patch set is merged. Rebasing not fun.

fosskers commented 3 years ago

No problem. I had a few other docs-related things I want to do in the pacman repo, but yeah I'm going to wait until yours are all in.

Between the two of us we can probably get the docs both there and here looking pretty nice.