On Mon, Jul 6, 2009 at 14:42, Martin Mares<mj@xxxxxx> wrote:
>> #elif defined(PCI_HAVE_STDINT_H) || (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L)
>> -#include <stdint.h>
>> +#include <inttypes.h>
>
> Even though it is probably OK to assume that <inttypes.h> exists whenever
> <stdint.h> does, keeping using PCI_HAVE_STDINT_H for that is confusing :)
Are you suggesting just removing the defined(PCI_HAVE_STDINT_H) part?
Also, I looked at the configure script and it looks like
PCI_HAVE_STDINT_H is only defined on sunos. That doesn't make much
since to me. There are only two references to that symbol in all the
source code. One in the lib/configure script and one in types.h. Shall
I just remove them both?
>> #else
>> +#include <inttypes.h>
>> typedef u_int8_t u8;
>> typedef u_int16_t u16;
>> typedef u_int32_t u32;
>> +typedef u_int64_t u64;
>
> As I already said, we cannot rely on <inttypes.h> on non-C99 systems.
For non-MS Windows systems, what systems do we support that don't have
those c99 headers?
I found that AIX 4.x, which has been end of lifed for a while, doesn't
have the headers, but AIX 5.3 does. Do we really need to maintain
support of systems as old as AIX 4.x? Could we take a more pragmatic
approach and introduce a change that may break the platform if I
promise to work with a user to get it building on AIX 4 if they really
need it. I doubt that and AIX 4 user would need new pciutils anyhow.
Also, do we have a list of supported platforms somewhere?
> Also, we have to define the printf sequences ourselves.
True. I hadn't solved that one yet. :(
>> #ifdef PCI_HAVE_64BIT_ADDRESS
>> #include <limits.h>
>> #if ULONG_MAX > 0xffffffff
>> -typedef unsigned long u64;
>> -#define PCI_U64_FMT "l"
>> +typedef unsigned long pciaddr_t;
>> #else
>> -typedef unsigned long long u64;
>> -#define PCI_U64_FMT "ll"
>> -#endif
>> +typedef unsigned long long pciaddr_t;
>> #endif
>>
>> -#endif /* PCI_HAVE_Uxx_TYPES */
>> +#define PCIADDR_T_FMT "%08" PRIu64 "x"
>> +#define PCIADDR_PORT_FMT "%04" PRIu64 "x"
>
> No, this is wrong. Since pciaddr_t is no longer u64, we must not use u64
> printf sequences for it.
This is reasonable.
>> - const char * const formats[] = { NULL, " %02x", " %04x", NULL, " %08x" };
>> - const char * const mask_formats[] = { NULL, " %02x->(%02x:%02x)->%02x", " %04x->(%04x:%04x)->%04x", NULL, " %08x->(%08x:%08x)->%08x" };
>> - unsigned int i, x, y;
>> + const char * const formats[] = { NULL, " %02x", " %04x", NULL, " %08x", NULL, NULL, NULL, " %016" PRIu64 "x"};
>> + const char * const mask_formats[] = { NULL, " %02x->(%02x:%02x)->%02x", " %04x->(%04x:%04x)->%04x", NULL, " %08x->(%08x:%08x)->%08x", NULL, NULL, NULL, " %016" PRIu64 "x->(%016" PRIu64 "x:%016" PRIu64 "x)->%016" PRIu64 "x"};
>> + unsigned int i;
>> + u64 x, y;
>> int addr = 0;
>> int width = op->width;
>> char slot[16];
>> @@ -120,6 +121,9 @@ exec_op(struct op *op, struct pci_dev *dev)
>> case 2:
>> y = pci_read_word(dev, addr);
>> break;
>> + case 8:
>> + y = pci_read_quad(dev, addr);
>> + break;
>
> This is also wrong. You have changed x,y to u64, but when you read a smaller
> value to them, you try to print them with a non-u64 format string.
I don't understand this comment fully. Previously, these values were
u32. Didn't they get printed the same way, that is using the non-u32
format strings?
Thanks,
wt
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
