Add libc_bitflags convenience macro #315

kamalmarhubi · 2016-03-15T18:16:51Z

We define many bitflags types with values from the libc crate. Currently
these look like this:

bitflags!{
    flags ProtFlags: libc::c_int {
        const PROT_NONE      = libc::PROT_NONE,
        const PROT_READ      = libc::PROT_READ,
        const PROT_WRITE     = libc::PROT_WRITE,
        const PROT_EXEC      = libc::PROT_EXEC,
        #[cfg(any(target_os = "linux", target_os = "android"))]
        const PROT_GROWSDOWN = libc::PROT_GROWSDOWN,
        #[cfg(any(target_os = "linux", target_os = "android"))]
        const PROT_GROWSUP   = libc::PROT_GROWSUP,
    }
}

There's some repetition which is tedious. With the new macro, the above
can instead be written

libc_bitflags!{
    flags ProtFlags: libc::c_int {
        PROT_NONE,
        PROT_READ,
        PROT_WRITE,
        PROT_EXEC,
        #[cfg(any(target_os = "linux", target_os = "android"))]
        PROT_GROWSDOWN,
        #[cfg(any(target_os = "linux", target_os = "android"))]
        PROT_GROWSUP,
    }
}

Thanks to Daniel Keep for the Little Book of Rust Macros, and for
helping with this macro.

Refs #264

kamalmarhubi · 2016-03-15T18:17:25Z

r? @fiveop @posborne

fiveop · 2016-03-16T06:03:48Z

src/macros.rs

+///
+/// # Example
+/// ```
+/// # #[macro_use] extern crate nix;


I would not make this publicly available.

ah this was me misunderstanding how macro visibility worked. good catch!

fiveop · 2016-03-16T06:07:04Z

I like this change.

Can you change all occurences instead of just one, right away. It should not be so many.

We define many bitflags types with values from the libc crate. Currently these look like this: bitflags!{ flags ProtFlags: libc::c_int { const PROT_NONE = libc::PROT_NONE, const PROT_READ = libc::PROT_READ, const PROT_WRITE = libc::PROT_WRITE, const PROT_EXEC = libc::PROT_EXEC, #[cfg(any(target_os = "linux", target_os = "android"))] const PROT_GROWSDOWN = libc::PROT_GROWSDOWN, #[cfg(any(target_os = "linux", target_os = "android"))] const PROT_GROWSUP = libc::PROT_GROWSUP, } } There's some repetition which is tedious. With the new macro, the above can instead be written libc_bitflags!{ flags ProtFlags: libc::c_int { PROT_NONE, PROT_READ, PROT_WRITE, PROT_EXEC, #[cfg(any(target_os = "linux", target_os = "android"))] PROT_GROWSDOWN, #[cfg(any(target_os = "linux", target_os = "android"))] PROT_GROWSUP, } } Thanks to Daniel Keep for the Little Book of Rust Macros, and for helping with this macro. Refs nix-rust#264

kamalmarhubi · 2016-03-16T16:16:02Z

Going through the rest now.

posborne · 2016-03-16T19:32:27Z

Got my approval as well, although I have not gone through the macros with a fine-tooth comb (but agreed with @fiveop on mot making the macro public). I would be fine with merging this even if we don't have 100% of libc flags converted.

Once you are happy with things, r=me is fine.

kamalmarhubi · 2016-03-16T19:54:43Z

I changed visibility of the macro, and added the use of the macro to the conventions doc.

I would be fine with merging this even if we don't have 100% of libc flags converted.

Having spent most of the morning today trying to convert ~60 uses with medium success, I'd much rather merge this and do the rest incrementally. The conversion should happen but isn't high priority.

Leaving this to one of you to r+ taking into account the conventions change.

posborne · 2016-03-16T20:01:11Z

Looks good to me. @homu r+

homu · 2016-03-16T20:01:13Z

📌 Commit c9e5a15 has been approved by posborne

homu · 2016-03-16T20:01:24Z

⌛ Testing commit c9e5a15 with merge 4715dde...

Add libc_bitflags convenience macro We define many bitflags types with values from the libc crate. Currently these look like this: bitflags!{ flags ProtFlags: libc::c_int { const PROT_NONE = libc::PROT_NONE, const PROT_READ = libc::PROT_READ, const PROT_WRITE = libc::PROT_WRITE, const PROT_EXEC = libc::PROT_EXEC, #[cfg(any(target_os = "linux", target_os = "android"))] const PROT_GROWSDOWN = libc::PROT_GROWSDOWN, #[cfg(any(target_os = "linux", target_os = "android"))] const PROT_GROWSUP = libc::PROT_GROWSUP, } } There's some repetition which is tedious. With the new macro, the above can instead be written libc_bitflags!{ flags ProtFlags: libc::c_int { PROT_NONE, PROT_READ, PROT_WRITE, PROT_EXEC, #[cfg(any(target_os = "linux", target_os = "android"))] PROT_GROWSDOWN, #[cfg(any(target_os = "linux", target_os = "android"))] PROT_GROWSUP, } } Thanks to Daniel Keep for the Little Book of Rust Macros, and for helping with this macro. Refs #264

This serves as an example use of the libc_bitflags macro.

kamalmarhubi · 2016-03-16T23:14:26Z

@homu retry 7038da0

kamalmarhubi · 2016-03-16T23:18:14Z

@homu retry

kamalmarhubi · 2016-03-16T23:19:41Z

One day I'll get the hang of @homu r+ 7038da0

homu · 2016-03-16T23:19:51Z

⌛ Testing commit 7038da0 with merge 431e470...

Add libc_bitflags convenience macro We define many bitflags types with values from the libc crate. Currently these look like this: bitflags!{ flags ProtFlags: libc::c_int { const PROT_NONE = libc::PROT_NONE, const PROT_READ = libc::PROT_READ, const PROT_WRITE = libc::PROT_WRITE, const PROT_EXEC = libc::PROT_EXEC, #[cfg(any(target_os = "linux", target_os = "android"))] const PROT_GROWSDOWN = libc::PROT_GROWSDOWN, #[cfg(any(target_os = "linux", target_os = "android"))] const PROT_GROWSUP = libc::PROT_GROWSUP, } } There's some repetition which is tedious. With the new macro, the above can instead be written libc_bitflags!{ flags ProtFlags: libc::c_int { PROT_NONE, PROT_READ, PROT_WRITE, PROT_EXEC, #[cfg(any(target_os = "linux", target_os = "android"))] PROT_GROWSDOWN, #[cfg(any(target_os = "linux", target_os = "android"))] PROT_GROWSUP, } } Thanks to Daniel Keep for the Little Book of Rust Macros, and for helping with this macro. Refs #264

homu · 2016-03-16T23:28:26Z

☀️ Test successful - status

kamalmarhubi · 2016-03-17T00:01:01Z

Oh I guess that should have been r=@posborne, ah well

fiveop reviewed Mar 16, 2016
View reviewed changes

This was referenced Mar 16, 2016

Add some constants to libc #316

Closed

Fix flag type names to match FOO_* => FooFlags convention #317

Open

kamalmarhubi force-pushed the libc-bitflags branch from 2a720bd to c9e5a15 Compare March 16, 2016 19:44

kamalmarhubi force-pushed the libc-bitflags branch 3 times, most recently from 18aa62f to c9a3dc8 Compare March 16, 2016 20:10

mman: Use libc_bitflags macro for ProtFlags

7038da0

This serves as an example use of the libc_bitflags macro.

kamalmarhubi force-pushed the libc-bitflags branch from c9a3dc8 to 7038da0 Compare March 16, 2016 20:11

homu merged commit 7038da0 into nix-rust:master Mar 16, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add libc_bitflags convenience macro #315

Add libc_bitflags convenience macro #315

kamalmarhubi commented Mar 15, 2016

kamalmarhubi commented Mar 15, 2016

fiveop Mar 16, 2016

kamalmarhubi Mar 16, 2016

fiveop commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

posborne commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

posborne commented Mar 16, 2016

homu commented Mar 16, 2016

homu commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

homu commented Mar 16, 2016

homu commented Mar 16, 2016

kamalmarhubi commented Mar 17, 2016

Add libc_bitflags convenience macro #315

Add libc_bitflags convenience macro #315

Conversation

kamalmarhubi commented Mar 15, 2016

kamalmarhubi commented Mar 15, 2016

fiveop Mar 16, 2016

Choose a reason for hiding this comment

kamalmarhubi Mar 16, 2016

Choose a reason for hiding this comment

fiveop commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

posborne commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

posborne commented Mar 16, 2016

homu commented Mar 16, 2016

homu commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

kamalmarhubi commented Mar 16, 2016

homu commented Mar 16, 2016

homu commented Mar 16, 2016

kamalmarhubi commented Mar 17, 2016