[PATCH 1/3] smbus: Cast ioctl argument

[Mike Waychison]

When building against klibc, iotools gets warnings about the third
argument to ioctl(2). This is due to glibc using a variadic function
for the call (to avoid casting and type errors), while klibc uses a
"void *" for the third argument.

Cast this arg in the two calls to ioctl. We use a cast to intptr_t to
avoid type size warnings that are emitted by gcc when the casting
directly to a "void *".

Signed-off-by: Mike Waychison <mi...@google.com>
---
smbus_rw.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/smbus_rw.c b/smbus_rw.c
index b0bd2e7..b54a4fe 100644
--- a/smbus_rw.c
+++ b/smbus_rw.c
@@ -74,7 +74,7 @@ open_i2c_slave(unsigned char i2c_bus, unsigned char slave_address)
return -1;
}

- if (ioctl(fd, I2C_SLAVE, slave_address) < 0) {
+ if (ioctl(fd, I2C_SLAVE, (void *)(intptr_t)slave_address) < 0) {
printf("Could not attach to i2c bus %d slave address %d: %s\n",
i2c_bus, slave_address, strerror(errno));
close(fd);
@@ -315,7 +315,8 @@ smbus_write_op(struct smbus_op_params *params, const struct smbus_op *op)

/* FIXME: Why is this needed if the open_i2c_slave performs the ioctl
* with I2C_SLAVE? */
- if (ioctl(params->fd, I2C_SLAVE_FORCE, params->address) < 0) {
+ if (ioctl(params->fd, I2C_SLAVE_FORCE,
+ (void *)(intptr_t)params->address) < 0) {
fprintf(stderr, "can't set address 0x%02X, %s\n",
params->address, strerror(errno));
return -1;
--
1.7.3.1

[Mike Waychison]

sched_setaffinity is a Linux-ism, and cpu_set_t is a glibc-ism. As
such, the kernel is expecting a pointer to an array of unsigned long,
while glibc uses a different type for the prototype.

As well, CPU_ZERO and CPU_SET are more glibc-isms that aren't available
in klibc.

Handle this by providing implementations for CPU_ZERO, CPU_SET, the
cpu_set_t typedef and handle the difference in sched_setaffinity
prototype when they aren't available, such as when building against
klibc.

Signed-off-by: Mike Waychison <mi...@google.com>
---

misc.c | 35 ++++++++++++++++++++++++++++++++++-
1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/misc.c b/misc.c
index 409c3a8..0506995 100644
--- a/misc.c
+++ b/misc.c
@@ -32,6 +32,39 @@
#include <sched.h>
#include "commands.h"

+/*
+ * There is a chance that we don't have cpu_set_t available to us, like
+ * in the case where we are building against klibc. In these cases,
+ * dummy up the glibc interfaces.
+ */
+#ifndef __CPU_SETSIZE
+#define __CPU_SETSIZE 1024
+#define __NCPUBITS (8 * sizeof (__cpu_mask))
+typedef unsigned long int __cpu_mask;
+
+/* Data structure to describe CPU mask. */
+typedef struct
+{
+ __cpu_mask __bits[__CPU_SETSIZE / __NCPUBITS];
+} cpu_set_t;
+
+static void CPU_ZERO(cpu_set_t *set) {
+ memset(set, 0, sizeof(set));
+}
+static void CPU_SET(int cpu, cpu_set_t *set) {
+ set->__bits[cpu / __NCPUBITS] |= 1 << (cpu % __NCPUBITS);
+}
+
+/* sched_setaffinity actually takes an "unsigned long *" for the mask,
+ * while glibc takes a "cpu_set_t *". Wrap the call to deal with the
+ * inconsistency. */
+static int local_setaffinity(pid_t pid, size_t cpusetsize, cpu_set_t *mask) {
+ return sched_setaffinity(pid, cpusetsize, (unsigned long *)mask);
+}
+#else
+#define local_setaffinity sched_setaffinity
+#endif
+
/* Helper function to set the affinity of the process to a given cpu. */
static int
set_cpu_affinity(int cpu)
@@ -41,7 +74,7 @@ set_cpu_affinity(int cpu)
/* run on the specified CPU */
CPU_ZERO(&cpuset);
CPU_SET(cpu, &cpuset);
- if (sched_setaffinity(getpid(), sizeof(cpuset), &cpuset) < 0) {
+ if (local_setaffinity(getpid(), sizeof(cpuset), &cpuset) < 0) {
perror("sched_setaffinity()");
return -1;
}
--
1.7.3.1

[Mike Waychison]

sysconf is from POSIX.1-2001, but may not be available in all libc
implementations.

If we detect that the _SC_NPROCESSORS_ONLN isn't available, let the
cpu_list command be unimplemented.

Signed-off-by: Mike Waychison <mi...@google.com>
---

misc.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/misc.c b/misc.c
index 0506995..b6a0e81 100644
--- a/misc.c
+++ b/misc.c
@@ -183,6 +183,7 @@ cpuid(int argc, const char *argv[], const struct cmd_info *info)
static int
cpu_list(int argc, const char *argv[], const struct cmd_info *info)
{
+#ifdef _SC_NPROCESSORS_ONLN
int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
int i;

@@ -190,6 +191,10 @@ cpu_list(int argc, const char *argv[], const struct cmd_info *info)
printf("%d\n", i);
}
return 0;
+#else
+ fprintf(stderr, "Operation not supported by library\n");
+ return -1;
+#endif
}

/*
--
1.7.3.1

[Mike Waychison]

The following three patches deal with inconsistencies I found while
compiling iotools against klibc 1.5.23. With this patchset applied (as
well as the pending patch to add the sched_setaffinity stub to klibc
here: http://www.zytor.com/pipermail/klibc/2011-June/002936.html), I can
successfully build iotools without warnings.

Diffstat
========

misc.c | 40 +++++++++++++++++++++++++++++++++++++++-
smbus_rw.c | 5 +++--
2 files changed, 42 insertions(+), 3 deletions(-)

[Tim Hockin]

I wonder if we can make this cleaner. Rather than fiddling with every
ioctl() call-site to add non-obvious casting, what about defining

int ioctl_wrapper(int fd, int request, ...) {
// void * arg = va_arg(...)
// call ioctl(fd, request, (void *)(intptr_t)arg);
}

Is that better or worse? :)

Why the double cast, BTW?

Tim

> --
> You received this message because you are subscribed to the Google Groups "iotools-devel" group.
> To post to this group, send email to iotool...@googlegroups.com.
> To unsubscribe from this group, send email to iotools-deve...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/iotools-devel?hl=en.
>
>

[Tim Hockin]

LGTM with minor formatting. Will apply.

On Mon, Jun 20, 2011 at 6:54 PM, Mike Waychison <mi...@google.com> wrote:

[Tim Hockin]

LGTM.

Puke.

On Mon, Jun 20, 2011 at 6:54 PM, Mike Waychison <mi...@google.com> wrote:

[Mike Waychison]

On Jun 20, 2011 8:07 PM, "Tim Hockin" <tho...@hockin.org> wrote:
>
> I wonder if we can make this cleaner.  Rather than fiddling with every
> ioctl() call-site to add non-obvious casting, what about defining
>
> int ioctl_wrapper(int fd, int request, ...) {
>  // void * arg =  va_arg(...)
>  // call ioctl(fd, request, (void *)(intptr_t)arg);
> }
>
> Is that better or worse? :)

Um, its not a hell of a lot better :(  I wonder if we can get Klibc to make this system call stub variadic.

>
> Why the double cast, BTW?

From the description:

"We use a cast to intptr_t to avoid type size warnings that are emitted by gcc when the casting directly to a "void *"."

In short, gcc doesn't like it when you cast a char to a pointer due to the size difference.  The cast to intptr_t masks that warning and says "yes, I know this is crazy" :-)

[Tim Hockin]

On Mon, Jun 20, 2011 at 9:21 PM, Mike Waychison <mi...@google.com> wrote:
>
> On Jun 20, 2011 8:07 PM, "Tim Hockin" <tho...@hockin.org> wrote:
>>
>> I wonder if we can make this cleaner.  Rather than fiddling with every
>> ioctl() call-site to add non-obvious casting, what about defining
>>
>> int ioctl_wrapper(int fd, int request, ...) {
>>  // void * arg =  va_arg(...)
>>  // call ioctl(fd, request, (void *)(intptr_t)arg);
>> }
>>
>> Is that better or worse? :)
>
> Um, its not a hell of a lot better :(  I wonder if we can get Klibc to make
> this system call stub variadic.

Yeah, F it. Committing.