From 5ff6ac4287e191ee684f1de1af642e7b656947b6 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 12 Nov 2021 12:58:38 -0800 Subject: [PATCH] Refactor weak symbols in std::sys::unix This makes a few changes to the weak symbol macros in `sys::unix`: - `dlsym!` is added to keep the functionality for runtime `dlsym` lookups, like for `__pthread_get_minstack@GLIBC_PRIVATE` that we don't want to show up in ELF symbol tables. - `weak!` now uses `#[linkage = "extern_weak"]` symbols, so its runtime behavior is just a simple null check. This is also used by `syscall!`. - On non-ELF targets (macos/ios) where that linkage is not known to behave, `weak!` is just an alias to `dlsym!` for the old behavior. - `raw_syscall!` is added to always call `libc::syscall` on linux and android, for cases like `clone3` that have no known libc wrapper. The new `weak!` linkage does mean that you'll get versioned symbols if you build with a newer glibc, like `WEAK DEFAULT UND statx@GLIBC_2.28`. This might seem problematic, but old non-weak symbols can tie the build to new versions too, like `dlsym@GLIBC_2.34` from their recent library unification. If you build with an old glibc like `dist-x86_64-linux` does, you'll still get unversioned `WEAK DEFAULT UND statx`, which may be resolved based on the runtime glibc. I also found a few functions that don't need to be weak anymore: - Android can directly use `ftruncate64`, `pread64`, and `pwrite64`, as these were added in API 12, and our baseline is API 14. - Linux can directly use `splice`, added way back in glibc 2.5 and similarly old musl. Android only added it in API 21 though. --- library/std/src/sys/unix/android.rs | 90 +-------------- library/std/src/sys/unix/fd.rs | 48 ++------ library/std/src/sys/unix/fs.rs | 20 ++-- library/std/src/sys/unix/kernel_copy.rs | 6 + library/std/src/sys/unix/net.rs | 4 +- library/std/src/sys/unix/os.rs | 24 ++-- library/std/src/sys/unix/os/tests.rs | 10 +- .../std/src/sys/unix/process/process_unix.rs | 4 +- library/std/src/sys/unix/thread.rs | 17 +-- library/std/src/sys/unix/weak.rs | 106 ++++++++++++++---- 10 files changed, 136 insertions(+), 193 deletions(-) diff --git a/library/std/src/sys/unix/android.rs b/library/std/src/sys/unix/android.rs index 6a46525f682..73ff10ab8a2 100644 --- a/library/std/src/sys/unix/android.rs +++ b/library/std/src/sys/unix/android.rs @@ -18,11 +18,9 @@ #![cfg(target_os = "android")] -use libc::{c_int, c_void, sighandler_t, size_t, ssize_t}; -use libc::{ftruncate, pread, pwrite}; +use libc::{c_int, sighandler_t}; -use super::{cvt, cvt_r, weak::weak}; -use crate::io; +use super::weak::weak; // The `log2` and `log2f` functions apparently appeared in android-18, or at // least you can see they're not present in the android-17 header [1] and they @@ -81,87 +79,3 @@ pub unsafe fn signal(signum: c_int, handler: sighandler_t) -> sighandler_t { let f = f.expect("neither `signal` nor `bsd_signal` symbols found"); f(signum, handler) } - -// The `ftruncate64` symbol apparently appeared in android-12, so we do some -// dynamic detection to see if we can figure out whether `ftruncate64` exists. -// -// If it doesn't we just fall back to `ftruncate`, generating an error for -// too-large values. -#[cfg(target_pointer_width = "32")] -pub fn ftruncate64(fd: c_int, size: u64) -> io::Result<()> { - weak!(fn ftruncate64(c_int, i64) -> c_int); - - unsafe { - match ftruncate64.get() { - Some(f) => cvt_r(|| f(fd, size as i64)).map(drop), - None => { - if size > i32::MAX as u64 { - Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"cannot truncate >2GB")) - } else { - cvt_r(|| ftruncate(fd, size as i32)).map(drop) - } - } - } - } -} - -#[cfg(target_pointer_width = "64")] -pub fn ftruncate64(fd: c_int, size: u64) -> io::Result<()> { - unsafe { cvt_r(|| ftruncate(fd, size as i64)).map(drop) } -} - -#[cfg(target_pointer_width = "32")] -pub unsafe fn cvt_pread64( - fd: c_int, - buf: *mut c_void, - count: size_t, - offset: i64, -) -> io::Result { - use crate::convert::TryInto; - weak!(fn pread64(c_int, *mut c_void, size_t, i64) -> ssize_t); - pread64.get().map(|f| cvt(f(fd, buf, count, offset))).unwrap_or_else(|| { - if let Ok(o) = offset.try_into() { - cvt(pread(fd, buf, count, o)) - } else { - Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"cannot pread >2GB")) - } - }) -} - -#[cfg(target_pointer_width = "32")] -pub unsafe fn cvt_pwrite64( - fd: c_int, - buf: *const c_void, - count: size_t, - offset: i64, -) -> io::Result { - use crate::convert::TryInto; - weak!(fn pwrite64(c_int, *const c_void, size_t, i64) -> ssize_t); - pwrite64.get().map(|f| cvt(f(fd, buf, count, offset))).unwrap_or_else(|| { - if let Ok(o) = offset.try_into() { - cvt(pwrite(fd, buf, count, o)) - } else { - Err(io::Error::new_const(io::ErrorKind::InvalidInput, &"cannot pwrite >2GB")) - } - }) -} - -#[cfg(target_pointer_width = "64")] -pub unsafe fn cvt_pread64( - fd: c_int, - buf: *mut c_void, - count: size_t, - offset: i64, -) -> io::Result { - cvt(pread(fd, buf, count, offset)) -} - -#[cfg(target_pointer_width = "64")] -pub unsafe fn cvt_pwrite64( - fd: c_int, - buf: *const c_void, - count: size_t, - offset: i64, -) -> io::Result { - cvt(pwrite(fd, buf, count, offset)) -} diff --git a/library/std/src/sys/unix/fd.rs b/library/std/src/sys/unix/fd.rs index 0956726084e..ea688571a98 100644 --- a/library/std/src/sys/unix/fd.rs +++ b/library/std/src/sys/unix/fd.rs @@ -99,30 +99,18 @@ impl FileDesc { } pub fn read_at(&self, buf: &mut [u8], offset: u64) -> io::Result { - #[cfg(target_os = "android")] - use super::android::cvt_pread64; - - #[cfg(not(target_os = "android"))] - unsafe fn cvt_pread64( - fd: c_int, - buf: *mut c_void, - count: usize, - offset: i64, - ) -> io::Result { - #[cfg(not(target_os = "linux"))] - use libc::pread as pread64; - #[cfg(target_os = "linux")] - use libc::pread64; - cvt(pread64(fd, buf, count, offset)) - } + #[cfg(not(any(target_os = "linux", target_os = "android")))] + use libc::pread as pread64; + #[cfg(any(target_os = "linux", target_os = "android"))] + use libc::pread64; unsafe { - cvt_pread64( + cvt(pread64( self.as_raw_fd(), buf.as_mut_ptr() as *mut c_void, cmp::min(buf.len(), READ_LIMIT), offset as i64, - ) + )) .map(|n| n as usize) } } @@ -161,30 +149,18 @@ impl FileDesc { } pub fn write_at(&self, buf: &[u8], offset: u64) -> io::Result { - #[cfg(target_os = "android")] - use super::android::cvt_pwrite64; - - #[cfg(not(target_os = "android"))] - unsafe fn cvt_pwrite64( - fd: c_int, - buf: *const c_void, - count: usize, - offset: i64, - ) -> io::Result { - #[cfg(not(target_os = "linux"))] - use libc::pwrite as pwrite64; - #[cfg(target_os = "linux")] - use libc::pwrite64; - cvt(pwrite64(fd, buf, count, offset)) - } + #[cfg(not(any(target_os = "linux", target_os = "android")))] + use libc::pwrite as pwrite64; + #[cfg(any(target_os = "linux", target_os = "android"))] + use libc::pwrite64; unsafe { - cvt_pwrite64( + cvt(pwrite64( self.as_raw_fd(), buf.as_ptr() as *const c_void, cmp::min(buf.len(), READ_LIMIT), offset as i64, - ) + )) .map(|n| n as usize) } } diff --git a/library/std/src/sys/unix/fs.rs b/library/std/src/sys/unix/fs.rs index a4fff9b2e64..d77e5cae3ad 100644 --- a/library/std/src/sys/unix/fs.rs +++ b/library/std/src/sys/unix/fs.rs @@ -46,8 +46,8 @@ use libc::fstatat64; use libc::readdir_r as readdir64_r; #[cfg(target_os = "android")] use libc::{ - dirent as dirent64, fstat as fstat64, fstatat as fstatat64, lseek64, lstat as lstat64, - open as open64, stat as stat64, + dirent as dirent64, fstat as fstat64, fstatat as fstatat64, ftruncate64, lseek64, + lstat as lstat64, off64_t, open as open64, stat as stat64, }; #[cfg(not(any( target_os = "linux", @@ -835,16 +835,10 @@ impl File { } pub fn truncate(&self, size: u64) -> io::Result<()> { - #[cfg(target_os = "android")] - return crate::sys::android::ftruncate64(self.as_raw_fd(), size); - - #[cfg(not(target_os = "android"))] - { - use crate::convert::TryInto; - let size: off64_t = - size.try_into().map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; - cvt_r(|| unsafe { ftruncate64(self.as_raw_fd(), size) }).map(drop) - } + use crate::convert::TryInto; + let size: off64_t = + size.try_into().map_err(|e| io::Error::new(io::ErrorKind::InvalidInput, e))?; + cvt_r(|| unsafe { ftruncate64(self.as_raw_fd(), size) }).map(drop) } pub fn read(&self, buf: &mut [u8]) -> io::Result { @@ -1154,7 +1148,7 @@ pub fn link(original: &Path, link: &Path) -> io::Result<()> { } else if #[cfg(target_os = "macos")] { // On MacOS, older versions (<=10.9) lack support for linkat while newer // versions have it. We want to use linkat if it is available, so we use weak! - // to check. `linkat` is preferable to `link` ecause it gives us a flag to + // to check. `linkat` is preferable to `link` because it gives us a flag to // specify how symlinks should be handled. We pass 0 as the flags argument, // meaning it shouldn't follow symlinks. weak!(fn linkat(c_int, *const c_char, c_int, *const c_char, c_int) -> c_int); diff --git a/library/std/src/sys/unix/kernel_copy.rs b/library/std/src/sys/unix/kernel_copy.rs index a6b43229ba6..f5c791d5805 100644 --- a/library/std/src/sys/unix/kernel_copy.rs +++ b/library/std/src/sys/unix/kernel_copy.rs @@ -612,6 +612,9 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) -> static HAS_SENDFILE: AtomicBool = AtomicBool::new(true); static HAS_SPLICE: AtomicBool = AtomicBool::new(true); + // Android builds use feature level 14, but the libc wrapper for splice is + // gated on feature level 21+, so we have to invoke the syscall directly. + #[cfg(target_os = "android")] syscall! { fn splice( srcfd: libc::c_int, @@ -623,6 +626,9 @@ fn sendfile_splice(mode: SpliceMode, reader: RawFd, writer: RawFd, len: u64) -> ) -> libc::ssize_t } + #[cfg(target_os = "linux")] + use libc::splice; + match mode { SpliceMode::Sendfile if !HAS_SENDFILE.load(Ordering::Relaxed) => { return CopyResult::Fallback(0); diff --git a/library/std/src/sys/unix/net.rs b/library/std/src/sys/unix/net.rs index 9ae6d12dcb9..a82a0172126 100644 --- a/library/std/src/sys/unix/net.rs +++ b/library/std/src/sys/unix/net.rs @@ -501,7 +501,7 @@ impl FromRawFd for Socket { // res_init unconditionally, we call it only when we detect we're linking // against glibc version < 2.26. (That is, when we both know its needed and // believe it's thread-safe). -#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))] +#[cfg(all(target_os = "linux", target_env = "gnu"))] fn on_resolver_failure() { use crate::sys; @@ -513,5 +513,5 @@ fn on_resolver_failure() { } } -#[cfg(any(not(target_env = "gnu"), target_os = "vxworks"))] +#[cfg(not(all(target_os = "linux", target_env = "gnu")))] fn on_resolver_failure() {} diff --git a/library/std/src/sys/unix/os.rs b/library/std/src/sys/unix/os.rs index 87893d26912..7686b61b67a 100644 --- a/library/std/src/sys/unix/os.rs +++ b/library/std/src/sys/unix/os.rs @@ -2,7 +2,7 @@ #![allow(unused_imports)] // lots of cfg code here -#[cfg(all(test, target_env = "gnu"))] +#[cfg(test)] mod tests; use crate::os::unix::prelude::*; @@ -636,30 +636,22 @@ pub fn getppid() -> u32 { unsafe { libc::getppid() as u32 } } -#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))] +#[cfg(all(target_os = "linux", target_env = "gnu"))] pub fn glibc_version() -> Option<(usize, usize)> { - if let Some(Ok(version_str)) = glibc_version_cstr().map(CStr::to_str) { + extern "C" { + fn gnu_get_libc_version() -> *const libc::c_char; + } + let version_cstr = unsafe { CStr::from_ptr(gnu_get_libc_version()) }; + if let Ok(version_str) = version_cstr.to_str() { parse_glibc_version(version_str) } else { None } } -#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))] -fn glibc_version_cstr() -> Option<&'static CStr> { - weak! { - fn gnu_get_libc_version() -> *const libc::c_char - } - if let Some(f) = gnu_get_libc_version.get() { - unsafe { Some(CStr::from_ptr(f())) } - } else { - None - } -} - // Returns Some((major, minor)) if the string is a valid "x.y" version, // ignoring any extra dot-separated parts. Otherwise return None. -#[cfg(all(target_env = "gnu", not(target_os = "vxworks")))] +#[cfg(all(target_os = "linux", target_env = "gnu"))] fn parse_glibc_version(version: &str) -> Option<(usize, usize)> { let mut parsed_ints = version.split('.').map(str::parse::).fuse(); match (parsed_ints.next(), parsed_ints.next()) { diff --git a/library/std/src/sys/unix/os/tests.rs b/library/std/src/sys/unix/os/tests.rs index c445acf2722..efc29955b05 100644 --- a/library/std/src/sys/unix/os/tests.rs +++ b/library/std/src/sys/unix/os/tests.rs @@ -1,14 +1,12 @@ -use super::*; - #[test] -#[cfg(not(target_os = "vxworks"))] +#[cfg(all(target_os = "linux", target_env = "gnu"))] fn test_glibc_version() { // This mostly just tests that the weak linkage doesn't panic wildly... - glibc_version(); + super::glibc_version(); } #[test] -#[cfg(not(target_os = "vxworks"))] +#[cfg(all(target_os = "linux", target_env = "gnu"))] fn test_parse_glibc_version() { let cases = [ ("0.0", Some((0, 0))), @@ -20,6 +18,6 @@ fn test_parse_glibc_version() { ("foo.1", None), ]; for &(version_str, parsed) in cases.iter() { - assert_eq!(parsed, parse_glibc_version(version_str)); + assert_eq!(parsed, super::parse_glibc_version(version_str)); } } diff --git a/library/std/src/sys/unix/process/process_unix.rs b/library/std/src/sys/unix/process/process_unix.rs index 3bf1493f3b8..bce35b380e6 100644 --- a/library/std/src/sys/unix/process/process_unix.rs +++ b/library/std/src/sys/unix/process/process_unix.rs @@ -13,7 +13,7 @@ use crate::sys::process::process_common::*; use crate::os::linux::process::PidFd; #[cfg(target_os = "linux")] -use crate::sys::weak::syscall; +use crate::sys::weak::raw_syscall; #[cfg(any( target_os = "macos", @@ -162,7 +162,7 @@ impl Command { cgroup: u64, } - syscall! { + raw_syscall! { fn clone3(cl_args: *mut clone_args, len: libc::size_t) -> libc::c_long } diff --git a/library/std/src/sys/unix/thread.rs b/library/std/src/sys/unix/thread.rs index b99eb2e553f..9e02966b57c 100644 --- a/library/std/src/sys/unix/thread.rs +++ b/library/std/src/sys/unix/thread.rs @@ -7,7 +7,9 @@ use crate::ptr; use crate::sys::{os, stack_overflow}; use crate::time::Duration; -#[cfg(any(target_os = "linux", target_os = "solaris", target_os = "illumos"))] +#[cfg(all(target_os = "linux", target_env = "gnu"))] +use crate::sys::weak::dlsym; +#[cfg(any(target_os = "solaris", target_os = "illumos"))] use crate::sys::weak::weak; #[cfg(not(any(target_os = "l4re", target_os = "vxworks", target_os = "espidf")))] pub const DEFAULT_MIN_STACK_SIZE: usize = 2 * 1024 * 1024; @@ -627,10 +629,12 @@ pub mod guard { // We need that information to avoid blowing up when a small stack // is created in an application with big thread-local storage requirements. // See #6233 for rationale and details. -#[cfg(target_os = "linux")] -#[allow(deprecated)] +#[cfg(all(target_os = "linux", target_env = "gnu"))] fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { - weak!(fn __pthread_get_minstack(*const libc::pthread_attr_t) -> libc::size_t); + // We use dlsym to avoid an ELF version dependency on GLIBC_PRIVATE. (#23628) + // We shouldn't really be using such an internal symbol, but there's currently + // no other way to account for the TLS size. + dlsym!(fn __pthread_get_minstack(*const libc::pthread_attr_t) -> libc::size_t); match __pthread_get_minstack.get() { None => libc::PTHREAD_STACK_MIN, @@ -638,9 +642,8 @@ fn min_stack_size(attr: *const libc::pthread_attr_t) -> usize { } } -// No point in looking up __pthread_get_minstack() on non-glibc -// platforms. -#[cfg(all(not(target_os = "linux"), not(target_os = "netbsd")))] +// No point in looking up __pthread_get_minstack() on non-glibc platforms. +#[cfg(all(not(all(target_os = "linux", target_env = "gnu")), not(target_os = "netbsd")))] fn min_stack_size(_: *const libc::pthread_attr_t) -> usize { libc::PTHREAD_STACK_MIN } diff --git a/library/std/src/sys/unix/weak.rs b/library/std/src/sys/unix/weak.rs index ba432ec5494..32072affe8a 100644 --- a/library/std/src/sys/unix/weak.rs +++ b/library/std/src/sys/unix/weak.rs @@ -6,7 +6,7 @@ //! detection. //! //! One option to use here is weak linkage, but that is unfortunately only -//! really workable on Linux. Hence, use dlsym to get the symbol value at +//! really workable with ELF. Otherwise, use dlsym to get the symbol value at //! runtime. This is also done for compatibility with older versions of glibc, //! and to avoid creating dependencies on GLIBC_PRIVATE symbols. It assumes that //! we've been dynamically linked to the library the symbol comes from, but that @@ -14,7 +14,8 @@ //! //! A long time ago this used weak linkage for the __pthread_get_minstack //! symbol, but that caused Debian to detect an unnecessarily strict versioned -//! dependency on libc6 (#23628). +//! dependency on libc6 (#23628) because it is GLIBC_PRIVATE. We now use `dlsym` +//! for a runtime lookup of that symbol to avoid the ELF versioned dependency. // There are a variety of `#[cfg]`s controlling which targets are involved in // each instance of `weak!` and `syscall!`. Rather than trying to unify all of @@ -22,31 +23,75 @@ #![allow(dead_code, unused_macros)] use crate::ffi::CStr; -use crate::marker; +use crate::marker::PhantomData; use crate::mem; use crate::sync::atomic::{self, AtomicUsize, Ordering}; +// We can use true weak linkage on ELF targets. +#[cfg(not(any(target_os = "macos", target_os = "ios")))] pub(crate) macro weak { (fn $name:ident($($t:ty),*) -> $ret:ty) => ( - #[allow(non_upper_case_globals)] - static $name: crate::sys::weak::Weak $ret> = - crate::sys::weak::Weak::new(concat!(stringify!($name), '\0')); + let ref $name: ExternWeak $ret> = { + extern "C" { + #[linkage = "extern_weak"] + static $name: *const libc::c_void; + } + #[allow(unused_unsafe)] + ExternWeak::new(unsafe { $name }) + }; ) } -pub struct Weak { - name: &'static str, - addr: AtomicUsize, - _marker: marker::PhantomData, +// On non-ELF targets, use the dlsym approximation of weak linkage. +#[cfg(any(target_os = "macos", target_os = "ios"))] +pub(crate) use self::dlsym as weak; + +pub(crate) struct ExternWeak { + weak_ptr: *const libc::c_void, + _marker: PhantomData, } -impl Weak { - pub const fn new(name: &'static str) -> Weak { - Weak { name, addr: AtomicUsize::new(1), _marker: marker::PhantomData } +impl ExternWeak { + #[inline] + pub(crate) fn new(weak_ptr: *const libc::c_void) -> Self { + ExternWeak { weak_ptr, _marker: PhantomData } + } +} + +impl ExternWeak { + #[inline] + pub(crate) fn get(&self) -> Option { + unsafe { + if self.weak_ptr.is_null() { + None + } else { + Some(mem::transmute_copy::<*const libc::c_void, F>(&self.weak_ptr)) + } + } + } +} + +pub(crate) macro dlsym { + (fn $name:ident($($t:ty),*) -> $ret:ty) => ( + static DLSYM: DlsymWeak $ret> = + DlsymWeak::new(concat!(stringify!($name), '\0')); + let $name = &DLSYM; + ) +} + +pub(crate) struct DlsymWeak { + name: &'static str, + addr: AtomicUsize, + _marker: PhantomData, +} + +impl DlsymWeak { + pub(crate) const fn new(name: &'static str) -> Self { + DlsymWeak { name, addr: AtomicUsize::new(1), _marker: PhantomData } } - pub fn get(&self) -> Option { - assert_eq!(mem::size_of::(), mem::size_of::()); + #[inline] + pub(crate) fn get(&self) -> Option { unsafe { // Relaxed is fine here because we fence before reading through the // pointer (see the comment below). @@ -82,6 +127,8 @@ impl Weak { // Cold because it should only happen during first-time initalization. #[cold] unsafe fn initialize(&self) -> Option { + assert_eq!(mem::size_of::(), mem::size_of::()); + let val = fetch(self.name); // This synchronizes with the acquire fence in `get`. self.addr.store(val, Ordering::Release); @@ -105,14 +152,12 @@ unsafe fn fetch(name: &str) -> usize { pub(crate) macro syscall { (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( unsafe fn $name($($arg_name: $t),*) -> $ret { - use super::os; - weak! { fn $name($($t),*) -> $ret } if let Some(fun) = $name.get() { fun($($arg_name),*) } else { - os::set_errno(libc::ENOSYS); + super::os::set_errno(libc::ENOSYS); -1 } } @@ -123,11 +168,6 @@ pub(crate) macro syscall { pub(crate) macro syscall { (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( unsafe fn $name($($arg_name:$t),*) -> $ret { - use weak; - // This looks like a hack, but concat_idents only accepts idents - // (not paths). - use libc::*; - weak! { fn $name($($t),*) -> $ret } // Use a weak symbol from libc when possible, allowing `LD_PRELOAD` @@ -135,6 +175,10 @@ pub(crate) macro syscall { if let Some(fun) = $name.get() { fun($($arg_name),*) } else { + // This looks like a hack, but concat_idents only accepts idents + // (not paths). + use libc::*; + syscall( concat_idents!(SYS_, $name), $($arg_name),* @@ -143,3 +187,19 @@ pub(crate) macro syscall { } ) } + +#[cfg(any(target_os = "linux", target_os = "android"))] +pub(crate) macro raw_syscall { + (fn $name:ident($($arg_name:ident: $t:ty),*) -> $ret:ty) => ( + unsafe fn $name($($arg_name:$t),*) -> $ret { + // This looks like a hack, but concat_idents only accepts idents + // (not paths). + use libc::*; + + syscall( + concat_idents!(SYS_, $name), + $($arg_name),* + ) as $ret + } + ) +}