Discussion:
[Ipmitool-devel] Series 1
Dan Gora
2013-03-21 23:37:47 UTC
Permalink
Here are the patches for the first series.

dan
Dan Gora
2013-03-21 23:49:48 UTC
Permalink
Fixed several bugs with printing out the Carrier and AMC connectivity
information with 'ipmitool -v fru'

There were several bugs that were caused due to using bitfields with
char types in ipmi_fru.h. GCC versions before v4.4 had a bug where
they did not pack the bitfields correctly if the bitfields were not
cleanly aligned with the underlying char type.

Fixed and cleaned up all other PICMG FRU records.

Signed-off-by: Dan Gora <***@adax.com>
---
ipmitool/include/ipmitool/ipmi_fru.h | 82 ++--
ipmitool/lib/ipmi_fru.c | 866 +++++++++++++++++++---------------
2 files changed, 526 insertions(+), 422 deletions(-)

diff --git a/ipmitool/include/ipmitool/ipmi_fru.h b/ipmitool/include/ipmitool/ipmi_fru.h
index b69e278..6833dd4 100644
--- a/ipmitool/include/ipmitool/ipmi_fru.h
+++ b/ipmitool/include/ipmitool/ipmi_fru.h
@@ -397,20 +397,21 @@ struct fru_picmgext_amc_link_desc {
#define OEM_SWFW_NBLOCK_OFFSET 0x05
#define OEM_SWFW_FIELD_START_OFFSET 0x06

+#define FRU_PICMGEXT_CHN_DESC_RECORD_SIZE 3
#ifdef HAVE_PRAGMA_PACK
#pragma pack(1)
#endif
struct fru_picmgext_chn_desc {
#ifndef WORDS_BIGENDIAN
- unsigned char remote_slot:8;
- unsigned char remote_chn:5;
- unsigned char local_chn:5;
- unsigned char:6;
+ unsigned int remote_slot:8;
+ unsigned int remote_chn:5;
+ unsigned int local_chn:5;
+ unsigned int res:14;
#else
- unsigned char:6;
- unsigned char local_chn:5;
- unsigned char remote_chn:5;
- unsigned char remote_slot:8;
+ unsigned int res:14;
+ unsigned int local_chn:5;
+ unsigned int remote_chn:5;
+ unsigned int remote_slot:8;
#endif
}ATTRIBUTE_PACKING;
#ifdef HAVE_PRAGMA_PACK
@@ -507,28 +508,30 @@ struct fru_picmgext_amc_p2p_record {
#pragma pack(0)
#endif

+#define FRU_PICMGEXT_AMC_CHANNEL_DESC_RECORD_SIZE 3
#ifdef HAVE_PRAGMA_PACK
#pragma pack(1)
#endif
struct fru_picmgext_amc_channel_desc_record {
#ifndef WORDS_BIGENDIAN
- unsigned char lane0port :5;
- unsigned char lane1port :5;
- unsigned char lane2port :5;
- unsigned char lane3port :5;
- unsigned char /*reserved */ :4;
-#else
- unsigned char /*reserved */ :4;
- unsigned char lane3port :5;
- unsigned char lane2port :5;
- unsigned char lane1port :5;
- unsigned char lane0port :5;
+ unsigned int lane0port :5;
+ unsigned int lane1port :5;
+ unsigned int lane2port :5;
+ unsigned int lane3port :5;
+ unsigned int /* reserved */ :12;
+#else
+ unsigned int /* reserved */ :12;
+ unsigned int lane3port :5;
+ unsigned int lane2port :5;
+ unsigned int lane1port :5;
+ unsigned int lane0port :5;
#endif
}ATTRIBUTE_PACKING;
#ifdef HAVE_PRAGMA_PACK
#pragma pack(0)
#endif

+#define FRU_PICMGEXT_AMC_LINK_DESC_RECORD_SIZE 5
#ifdef HAVE_PRAGMA_PACK
#pragma pack(1)
#endif
@@ -552,27 +555,28 @@ struct fru_picmgext_amc_link_desc_record {
#define AMC_LINK_TYPE_EXT_STORAGE_SATA 0x01
#define AMC_LINK_TYPE_EXT_STORAGE_SAS 0x02
#ifndef WORDS_BIGENDIAN
- unsigned short channel_id :8;
- unsigned short port_flag_0 :1;
- unsigned short port_flag_1 :1;
- unsigned short port_flag_2 :1;
- unsigned short port_flag_3 :1;
- unsigned short type :8;
- unsigned short type_ext :4;
- unsigned short group_id :8;
- unsigned char asym_match :2;
- unsigned char /* reserved */ :6;
+ unsigned int channel_id :8;
+ unsigned int port_flag_0 :1;
+ unsigned int port_flag_1 :1;
+ unsigned int port_flag_2 :1;
+ unsigned int port_flag_3 :1;
+ unsigned int type :8;
+ unsigned int type_ext :4;
+ unsigned int group_id :8;
+ unsigned int asym_match :2;
+ unsigned int /* reserved */ :30;
#else
- unsigned char /* reserved */ :6;
- unsigned char asym_match :2;
- unsigned short group_id :8;
- unsigned short type_ext :4;
- unsigned short type :8;
- unsigned short port_flag_3 :1;
- unsigned short port_flag_2 :1;
- unsigned short port_flag_1 :1;
- unsigned short port_flag_0 :1;
- unsigned short channel_id :8;
+ unsigned int group_id :8;
+ unsigned int type_ext :4;
+ unsigned int type :8;
+ unsigned int port_flag_3 :1;
+ unsigned int port_flag_2 :1;
+ unsigned int port_flag_1 :1;
+ unsigned int port_flag_0 :1;
+ unsigned int channel_id :8;
+
+ unsigned int /* reserved */ :30;
+ unsigned int asym_match :2;
#endif
}ATTRIBUTE_PACKING;
#ifdef HAVE_PRAGMA_PACK
diff --git a/ipmitool/lib/ipmi_fru.c b/ipmitool/lib/ipmi_fru.c
index 059c06f..7af3e5b 100644
--- a/ipmitool/lib/ipmi_fru.c
+++ b/ipmitool/lib/ipmi_fru.c
@@ -2133,7 +2133,7 @@ static int ipmi_fru_picmg_ext_edit(uint8_t * fru_data,
static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
{
struct fru_multirec_oem_header *h;
- int guid_count;
+ int guid_count;
int offset = off;
int start_offset = off;
int i;
@@ -2143,74 +2143,103 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)

switch (h->record_id)
{
-
case FRU_PICMG_BACKPLANE_P2P:
{
uint8_t index;
- struct fru_picmgext_slot_desc * slot_d
- = (struct fru_picmgext_slot_desc*) &fru_data[offset];
+ unsigned int data;
+ struct fru_picmgext_slot_desc *slot_d;

+ slot_d =
+ (struct fru_picmgext_slot_desc*) &fru_data[offset];
offset += sizeof(struct fru_picmgext_slot_desc);
printf(" FRU_PICMG_BACKPLANE_P2P\n");

while (offset <= (start_offset+length)) {
printf("\n");
printf(" Channel Type: ");
- switch ( slot_d -> chan_type )
+ switch (slot_d->chan_type)
{
- case 0x00:
- case 0x07:
- printf("PICMG 2.9\n");
- break;
- case 0x08:
- printf("Single Port Fabric IF\n");
- break;
- case 0x09:
- printf("Double Port Fabric IF\n");
- break;
- case 0x0a:
- printf("Full Channel Fabric IF\n");
- break;
- case 0x0b:
- printf("Base IF\n");
- break;
- case 0x0c:
- printf("Update Channel IF\n");
- break;
- default:
- printf("Unknown IF\n");
- break;
+ case 0x00:
+ case 0x07:
+ printf("PICMG 2.9\n");
+ break;
+ case 0x08:
+ printf("Single Port Fabric IF\n");
+ break;
+ case 0x09:
+ printf("Double Port Fabric IF\n");
+ break;
+ case 0x0a:
+ printf("Full Channel Fabric IF\n");
+ break;
+ case 0x0b:
+ printf("Base IF\n");
+ break;
+ case 0x0c:
+ printf("Update Channel IF\n");
+ break;
+ case 0x0d:
+ printf("ShMC Cross Connect\n");
+ break;
+ default:
+ printf("Unknown IF (0x%x)\n",
+ slot_d->chan_type);
+ break;
}
- printf(" Slot Addr. : %02x\n", slot_d -> slot_addr );
- printf(" Channel Count: %i\n", slot_d -> chn_count);
+ printf(" Slot Addr. : %02x\n",
+ slot_d->slot_addr);
+ printf(" Channel Count: %i\n",
+ slot_d->chn_count);

- for (index = 0; index < (slot_d -> chn_count); index++) {
- struct fru_picmgext_chn_desc * d
- = (struct fru_picmgext_chn_desc *) &fru_data[offset];
+ for (index = 0;
+ index < (slot_d -> chn_count);
+ index++) {
+ struct fru_picmgext_chn_desc *d;
+ data = (fru_data[offset+0]) |
+ (fru_data[offset+1] << 8) |
+ (fru_data[offset+2] << 16);
+ d = (struct fru_picmgext_chn_desc *)
+ &data;

if (verbose)
- printf( " "
- "Chn: %02x -> "
- "Chn: %02x in "
- "Slot: %02x\n",
- d->local_chn, d->remote_chn, d->remote_slot);
+ printf( " "
+ "Local Chn: %02x -> "
+ "Remote Chn: %02x in "
+ "Remote Slot: %02x\n",
+ d->local_chn,
+ d->remote_chn,
+ d->remote_slot);

- offset += sizeof(struct fru_picmgext_chn_desc);
- }
+ offset += FRU_PICMGEXT_CHN_DESC_RECORD_SIZE;
+ }

- slot_d = (struct fru_picmgext_slot_desc*) &fru_data[offset];
- offset += sizeof(struct fru_picmgext_slot_desc);
- }
- }
+ slot_d = (struct fru_picmgext_slot_desc*)
+ &fru_data[offset];
+ offset += sizeof(struct fru_picmgext_slot_desc);
+ }
+ }
break;

case FRU_PICMG_ADDRESS_TABLE:
{
- unsigned char entries = 0;
- unsigned char i;
-
+ unsigned int hwaddr;
+ unsigned int sitetype;
+ unsigned int sitenum;
+ unsigned int entries;
+ unsigned int i;
+ char *picmg_site_type_strings[] = {
+ "AdvancedTCA Board",
+ "Power Entry",
+ "Shelf FRU Information",
+ "Dedicated ShMC",
+ "Fan Tray",
+ "Fan Filter Tray",
+ "Alarm",
+ "AdvancedMC Module",
+ "PMC",
+ "Rear Transition Module"};
+
printf(" FRU_PICMG_ADDRESS_TABLE\n");
-
printf(" Type/Len: 0x%02x\n", fru_data[offset++]);
printf(" Shelf Addr: ");
for (i=0;i<20;i++) {
@@ -2222,61 +2251,117 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
printf(" Addr Table Entries: 0x%02x\n", entries);

for (i=0; i<entries; i++) {
- printf(" HWAddr: 0x%02x - SiteNum: 0x%02x - SiteType: 0x%02x \n",
- fru_data[offset++], fru_data[offset++], fru_data[offset++]);
+ hwaddr = fru_data[offset];
+ sitenum = fru_data[offset + 1];
+ sitetype = fru_data[offset + 2];
+ printf(
+ " HWAddr: 0x%02x (0x%02x) SiteNum: %d SiteType: 0x%02x %s\n",
+ hwaddr, hwaddr * 2,
+ sitenum, sitetype,
+ (sitetype < 0xa) ?
+ picmg_site_type_strings[sitetype] :
+ "Reserved");
+ offset += 3;
}
}
break;

case FRU_PICMG_SHELF_POWER_DIST:
{
- unsigned char i,j;
- unsigned char feeds = 0;
-
+ unsigned int i,j;
+ unsigned int feeds;
+ unsigned int entries;
+ unsigned int maxext;
+ unsigned int maxint;
+ unsigned int minexp;
+ unsigned int feedcnt;
+ unsigned int hwaddr;
+ unsigned int id;
+
printf(" FRU_PICMG_SHELF_POWER_DIST\n");

feeds = fru_data[offset++];
- printf(" Number of Power Feeds: 0x%02x\n", feeds);
-
+ printf(
+ " Number of Power Feeds: 0x%02x\n",
+ feeds);
for (i=0; i<feeds; i++) {
- unsigned char entries;
-
- printf(" Max Ext Current: 0x%04x\n", fru_data[offset+0] | (fru_data[offset+1]<<8));
+ printf(" Feed %d:\n", i);
+ maxext = fru_data[offset] |
+ (fru_data[offset+1] << 8);
offset += 2;
- printf(" Max Int Current: 0x%04x\n", fru_data[offset+0] | (fru_data[offset+1]<<8));
+ maxint = fru_data[offset] |
+ (fru_data[offset+1] << 8);
offset += 2;
- printf(" Min Exp Voltage: 0x%02x\n", fru_data[offset++]);
+ minexp = fru_data[offset];
+ offset += 1;
+ entries = fru_data[offset];
+ offset += 1;
+
+ printf(
+ " Max External Current: %d.%d Amps (0x%04x)\n",
+ maxext / 10, maxext % 10, maxext);
+ if (maxint < 0xffff)
+ printf(
+ " Max Internal Current: %d.%d Amps (0x%04x)\n",
+ maxint / 10, maxint % 10,
+ maxint);
+ else
+ printf(
+ " Max Internal Current: Not Specified\n");

- entries = fru_data[offset++];
- printf(" Feed to FRU count: 0x%02x\n", entries);
+ if (minexp >= 0x48 && minexp <= 0x90)
+ printf(
+ " Min Expected Voltage: -%02d.%dV\n",
+ minexp / 2, (minexp % 2) * 5);
+ else
+ printf(
+ " Min Expected Voltage: -36V (actual invalid value 0x%x)\n",
+ 36, minexp);

+ printf(
+ " Feed to FRU count: 0x%02x\n",
+ entries);
+
for (j=0; j<entries; j++) {
- printf(" HW: 0x%02x", fru_data[offset++]);
- printf(" FRU ID: 0x%02x\n", fru_data[offset++]);
+ hwaddr = fru_data[offset++];
+ id = fru_data[offset++];
+ printf(
+ " FRU HW Addr: 0x%02x (0x%02x)",
+ hwaddr, hwaddr * 2);
+ printf(" FRU ID: 0x%02x\n",
+ id);
}
}
-
}
break;

case FRU_PICMG_SHELF_ACTIVATION:
{
- unsigned char i;
- unsigned char count = 0;
+ unsigned int i;
+ unsigned int count;

printf(" FRU_PICMG_SHELF_ACTIVATION\n");

- printf(" Allowance for FRU Act Readiness: 0x%02x\n", fru_data[offset++]);
-
+ printf(
+ " Allowance for FRU Act Readiness: 0x%02x\n",
+ fru_data[offset++]);
+
count = fru_data[offset++];
- printf(" FRU activation and Power Desc Cnt: 0x%02x\n", count);
-
+ printf(
+ " FRU activation and Power Desc Cnt: 0x%02x\n",
+ count);
+
for (i=0; i<count; i++) {
- printf(" HW Addr: 0x%02x ", fru_data[offset++]);
- printf(" FRU ID: 0x%02x ", fru_data[offset++]);
- printf(" Max FRU Power: 0x%04x ", fru_data[offset+0] | (fru_data[offset+1]<<8));
+ printf(" HW Addr: 0x%02x ",
+ fru_data[offset++]);
+ printf(" FRU ID: 0x%02x ",
+ fru_data[offset++]);
+ printf(" Max FRU Power: 0x%04x ",
+ fru_data[offset+0] |
+ (fru_data[offset+1]<<8));
offset += 2;
- printf(" Config: 0x%02x \n", fru_data[offset++]);
+ printf(" Config: 0x%02x \n",
+ fru_data[offset++]);
}
}
break;
@@ -2294,71 +2379,73 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
int j;
printf(" GUID [%2d]: 0x", i);

- for (j=0; j < sizeof(struct fru_picmgext_guid); j++) {
+ for (j=0; j < sizeof(struct fru_picmgext_guid);
+ j++) {
printf("%02x", fru_data[offset+j]);
}

printf("\n");
offset += sizeof(struct fru_picmgext_guid);
}
- printf("\n");
-
- for (; offset < off + length; offset += sizeof(struct fru_picmgext_link_desc)) {
+ printf("\n");

+ for (; offset < off + length;
+ offset += sizeof(struct fru_picmgext_link_desc)) {
/* to solve little endian /big endian problem */
- unsigned long data = (fru_data[offset+0])
- | (fru_data[offset+1] << 8)
- | (fru_data[offset+2] << 16)
- | (fru_data[offset+3] << 24);
-
- struct fru_picmgext_link_desc * d = (struct fru_picmgext_link_desc *) &data;
+ struct fru_picmgext_link_desc *d;
+ unsigned int data = (fru_data[offset+0]) |
+ (fru_data[offset+1] << 8) |
+ (fru_data[offset+2] << 16) |
+ (fru_data[offset+3] << 24);
+ d = (struct fru_picmgext_link_desc *) &data;

- printf(" Link Grouping ID: 0x%02x\n", d->grouping);
- printf(" Link Type Extension: 0x%02x - ", d->ext);
- if (d->type == FRU_PICMGEXT_LINK_TYPE_BASE){
+ printf(" Link Grouping ID: 0x%02x\n",
+ d->grouping);
+ printf(" Link Type Extension: 0x%02x - ",
+ d->ext);
+ if (d->type == FRU_PICMGEXT_LINK_TYPE_BASE) {
switch (d->ext)
{
- case 0:
- printf("10/100/1000BASE-T Link (four-pair)\n");
- break;
- case 1:
- printf("ShMC Cross-connect (two-pair)\n");
- break;
- default:
- printf("Unknwon\n");
- break;
+ case 0:
+ printf("10/100/1000BASE-T Link (four-pair)\n");
+ break;
+ case 1:
+ printf("ShMC Cross-connect (two-pair)\n");
+ break;
+ default:
+ printf("Unknown\n");
+ break;
}
- }else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_ETHERNET){
+ } else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_ETHERNET){
switch (d->ext)
{
- case 0:
- printf("Fixed 1000Base-BX\n");
- break;
- case 1:
- printf("Fixed 10GBASE-BX4 [XAUI]\n");
- break;
- case 2:
- printf("FC-PI\n");
- break;
- default:
- printf("Unknwon\n");
- break;
+ case 0:
+ printf("Fixed 1000Base-BX\n");
+ break;
+ case 1:
+ printf("Fixed 10GBASE-BX4 [XAUI]\n");
+ break;
+ case 2:
+ printf("FC-PI\n");
+ break;
+ default:
+ printf("Unknown\n");
+ break;
}
-
+
}else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_INFINIBAND){
- printf("Unknwon\n");
+ printf("Unknown\n");
}else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_STAR){
- printf("Unknwon\n");
+ printf("Unknown\n");
}else if (d->type == FRU_PICMGEXT_LINK_TYPE_PCIE){
- printf("Unknwon\n");
- }else
- {
- printf("Unknwon\n");
+ printf("Unknown\n");
+ }else {
+ printf("Unknown\n");
}
-
- printf(" Link Type: 0x%02x - ",d->type);
- if (d->type == 0 || d->type == 0xff)
- {
+
+ printf(" Link Type: 0x%02x - ",
+ d->type);
+ if (d->type == 0 || d->type == 0xff) {
printf("Reserved\n");
}
else if (d->type >= 0x06 && d->type <= 0xef) {
@@ -2370,48 +2457,52 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
else {
switch (d->type)
{
- case FRU_PICMGEXT_LINK_TYPE_BASE:
- printf("PICMG 3.0 Base Interface 10/100/1000\n");
- break;
- case FRU_PICMGEXT_LINK_TYPE_FABRIC_ETHERNET:
- printf("PICMG 3.1 Ethernet Fabric Interface\n");
- break;
- case FRU_PICMGEXT_LINK_TYPE_FABRIC_INFINIBAND:
- printf("PICMG 3.2 Infiniband Fabric Interface\n");
- break;
- case FRU_PICMGEXT_LINK_TYPE_FABRIC_STAR:
- printf("PICMG 3.3 Star Fabric Interface\n");
- break;
- case FRU_PICMGEXT_LINK_TYPE_PCIE:
- printf("PICMG 3.4 PCI Express Fabric Interface\n");
- break;
- default:
- printf("Invalid\n");
- break;
+ case FRU_PICMGEXT_LINK_TYPE_BASE:
+ printf("PICMG 3.0 Base Interface 10/100/1000\n");
+ break;
+ case FRU_PICMGEXT_LINK_TYPE_FABRIC_ETHERNET:
+ printf("PICMG 3.1 Ethernet Fabric Interface\n");
+ break;
+ case FRU_PICMGEXT_LINK_TYPE_FABRIC_INFINIBAND:
+ printf("PICMG 3.2 Infiniband Fabric Interface\n");
+ break;
+ case FRU_PICMGEXT_LINK_TYPE_FABRIC_STAR:
+ printf("PICMG 3.3 Star Fabric Interface\n");
+ break;
+ case FRU_PICMGEXT_LINK_TYPE_PCIE:
+ printf("PICMG 3.4 PCI Express Fabric Interface\n");
+ break;
+ default:
+ printf("Invalid\n");
+ break;
}
}
printf(" Link Designator: \n");
- printf(" Port Flag: 0x%02x\n", d->desig_port);
- printf(" Interface: 0x%02x - ", d->desig_if);
- switch (d->desig_if)
- {
- case FRU_PICMGEXT_DESIGN_IF_BASE:
- printf("Base Interface\n");
- break;
- case FRU_PICMGEXT_DESIGN_IF_FABRIC:
- printf("Fabric Interface\n");
- break;
- case FRU_PICMGEXT_DESIGN_IF_UPDATE_CHANNEL:
- printf("Update Channel\n");
- break;
- case FRU_PICMGEXT_DESIGN_IF_RESERVED:
- printf("Reserved\n");
- break;
- default:
- printf("Invalid");
- break;
- }
- printf(" Channel Number: 0x%02x\n", d->desig_channel);
+ printf(" Port Flag: 0x%02x\n",
+ d->desig_port);
+ printf(
+ " Interface: 0x%02x - ",
+ d->desig_if);
+ switch (d->desig_if)
+ {
+ case FRU_PICMGEXT_DESIGN_IF_BASE:
+ printf("Base Interface\n");
+ break;
+ case FRU_PICMGEXT_DESIGN_IF_FABRIC:
+ printf("Fabric Interface\n");
+ break;
+ case FRU_PICMGEXT_DESIGN_IF_UPDATE_CHANNEL:
+ printf("Update Channel\n");
+ break;
+ case FRU_PICMGEXT_DESIGN_IF_RESERVED:
+ printf("Reserved\n");
+ break;
+ default:
+ printf("Invalid");
+ break;
+ }
+ printf(" Channel Number: 0x%02x\n",
+ d->desig_channel);
printf("\n");
}

@@ -2424,8 +2515,8 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)

current = fru_data[offset];
printf(" Current draw(@12V): %.2f A [ %.2f Watt ]\n",
- (float)current / 10.0f,
- (float)current / 10.0f * 12.0f);
+ (float)current / 10.0f,
+ (float)current / 10.0f * 12.0f);
printf("\n");
}
break;
@@ -2438,248 +2529,257 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
max_current = fru_data[offset];
max_current |= fru_data[++offset]<<8;
printf(" Maximum Internal Current(@12V): %.2f A [ %.2f Watt ]\n",
- (float)max_current / 10.0f,
- (float)max_current / 10.0f * 12.0f);
-
+ (float)max_current / 10.0f,
+ (float)max_current / 10.0f * 12.0f);
printf(" Module Activation Readiness: %i sec.\n", fru_data[++offset]);
printf(" Descriptor Count: %i\n", fru_data[++offset]);
printf("\n");

for(++offset; offset < off + length; offset += sizeof(struct fru_picmgext_activation_record))
{
- struct fru_picmgext_activation_record * a =
- (struct fru_picmgext_activation_record *) &fru_data[offset];
-
+ struct fru_picmgext_activation_record * a;
+ a = (struct fru_picmgext_activation_record *) &fru_data[offset];
printf(" IPMB-Address: 0x%x\n", a->ibmb_addr);
- printf(" Max. Module Current: %.2f A\n", (float)a->max_module_curr / 10.0f);
+ printf(" Max. Module Current: %.2f A\n",
+ (float)a->max_module_curr / 10.0f);
printf("\n");
}
}
break;

case FRU_AMC_CARRIER_P2P:
- printf(" FRU_CARRIER_P2P\n");
{
- uint16_t index;
-
- for(; offset < off + length; )
- {
- struct fru_picmgext_carrier_p2p_record * h =
- (struct fru_picmgext_carrier_p2p_record *) &fru_data[offset];
-
- printf("\n");
- printf(" Resource ID: %i", h->resource_id & 0x07);
- printf(" Type: ");
- if ((h->resource_id>>7) == 1) {
- printf("AMC\n");
- } else {
- printf("Local\n");
- }
- printf(" Descriptor Count: %i\n", h->p2p_count);
+ printf(" FRU_CARRIER_P2P\n");
+ uint16_t index;

- offset += sizeof(struct fru_picmgext_carrier_p2p_record);
+ for(; offset < off + length; ) {
+ struct fru_picmgext_carrier_p2p_record * h =
+ (struct fru_picmgext_carrier_p2p_record *) &fru_data[offset];

- for (index = 0; index < h->p2p_count; index++)
- {
- /* to solve little endian /big endian problem */
- unsigned char data[3];
- struct fru_picmgext_carrier_p2p_descriptor * desc;
+ printf("\n");
+ printf(" Resource ID: %i", h->resource_id & 0x07);
+ printf(" Type: ");
+ if ((h->resource_id>>7) == 1) {
+ printf("AMC\n");
+ } else {
+ printf("Local\n");
+ }
+ printf(" Descriptor Count: %i\n", h->p2p_count);

- #ifndef WORDS_BIGENDIAN
- data[0] = fru_data[offset+0];
- data[1] = fru_data[offset+1];
- data[2] = fru_data[offset+2];
- #else
- data[0] = fru_data[offset+2];
- data[1] = fru_data[offset+1];
- data[2] = fru_data[offset+0];
- #endif
+ offset += sizeof(struct fru_picmgext_carrier_p2p_record);

- desc = (struct fru_picmgext_carrier_p2p_descriptor*)&data;
+ for (index = 0; index < h->p2p_count; index++)
+ {
+ /* to solve little endian /big endian problem */
+ unsigned char data[3];
+ struct fru_picmgext_carrier_p2p_descriptor * desc;

- printf(" Port: %02d\t-> Remote Port: %02d\t",
- desc->local_port, desc->remote_port);
- if((desc->remote_resource_id >> 7) == 1)
- printf("[ AMC ID: %02d ]\n", desc->remote_resource_id & 0x0F);
- else
- printf("[ local ID: %02d ]\n", desc->remote_resource_id & 0x0F);
+ #ifndef WORDS_BIGENDIAN
+ data[0] = fru_data[offset+0];
+ data[1] = fru_data[offset+1];
+ data[2] = fru_data[offset+2];
+ #else
+ data[0] = fru_data[offset+2];
+ data[1] = fru_data[offset+1];
+ data[2] = fru_data[offset+0];
+ #endif
+
+ desc = (struct fru_picmgext_carrier_p2p_descriptor*)&data;
+
+ printf(" Port: %02d\t-> Remote Port: %02d\t",
+ desc->local_port, desc->remote_port);
+ if((desc->remote_resource_id >> 7) == 1)
+ printf("[ AMC ID: %02d ]\n", desc->remote_resource_id & 0x0F);
+ else
+ printf("[ local ID: %02d ]\n", desc->remote_resource_id & 0x0F);

- offset += sizeof(struct fru_picmgext_carrier_p2p_descriptor);
- }
+ offset += sizeof(struct fru_picmgext_carrier_p2p_descriptor);
}
}
+ }
break;

case FRU_AMC_P2P:
- printf(" FRU_AMC_P2P\n");
{
- unsigned int index;
- unsigned char channel_count;
- struct fru_picmgext_amc_p2p_record * h;
-
- guid_count = fru_data[offset++];
- printf(" GUID count: %2d\n", guid_count);
-
- for (i = 0 ; i < guid_count; i++ )
- {
- int j;
- printf(" GUID %2d: ", i);
-
- for (j=0; j < sizeof(struct fru_picmgext_guid); j++) {
- printf("%02x", fru_data[offset+j]);
- }
+ unsigned int index;
+ unsigned char channel_count;
+ struct fru_picmgext_amc_p2p_record * h;
+ printf(" FRU_AMC_P2P\n");

- offset += sizeof(struct fru_picmgext_guid);
- printf("\n");
+ guid_count = fru_data[offset];
+ printf(" GUID count: %2d\n", guid_count);
+
+ for (i = 0 ; i < guid_count; i++ )
+ {
+ int j;
+ printf(" GUID %2d: ", i);
+
+ for (j=0; j < sizeof(struct fru_picmgext_guid); j++) {
+ printf("%02x", fru_data[offset+j]);
}

- h = (struct fru_picmgext_amc_p2p_record *) &fru_data[offset];
- printf(" %s", (h->record_type?"AMC Module:":"On-Carrier Device"));
- printf(" Recource ID: %i\n", h->resource_id);
-
- offset += sizeof(struct fru_picmgext_amc_p2p_record);
-
- channel_count = fru_data[offset++];
- printf(" Descriptor Count: %i\n", channel_count);
-
- for (index=0 ;index < channel_count; index++)
- {
- struct fru_picmgext_amc_channel_desc_record * d =
- (struct fru_picmgext_amc_channel_desc_record *) &fru_data[offset];
-
- printf(" Lane 0 Port: %i\n", d->lane0port);
- printf(" Lane 1 Port: %i\n", d->lane1port);
- printf(" Lane 2 Port: %i\n", d->lane2port);
- printf(" Lane 3 Port: %i\n\n", d->lane3port);
-
+ offset += sizeof(struct fru_picmgext_guid);
+ printf("\n");
+ }
+
+ h = (struct fru_picmgext_amc_p2p_record *) &fru_data[++offset];
+ printf(" %s", (h->record_type?"AMC Module:":"On-Carrier Device"));
+ printf(" Resource ID: %i\n", h->resource_id);
+
+ offset += sizeof(struct fru_picmgext_amc_p2p_record);
+
+ channel_count = fru_data[offset++];
+ printf(" Descriptor Count: %i\n", channel_count);
+
+ for (index=0 ;index < channel_count; index++)
+ {
+ unsigned int data;
+ struct fru_picmgext_amc_channel_desc_record *d;
+
+ /* pack the data in little endian format.
+ * Stupid intel... */
+ data = fru_data[offset] |
+ (fru_data[offset + 1] << 8) |
+ (fru_data[offset + 2] << 16);
+ d = (struct fru_picmgext_amc_channel_desc_record *) &data;
+ printf(" Lane 0 Port: %i\n", d->lane0port);
+ printf(" Lane 1 Port: %i\n", d->lane1port);
+ printf(" Lane 2 Port: %i\n", d->lane2port);
+ printf(" Lane 3 Port: %i\n\n", d->lane3port);

- offset += sizeof(struct fru_picmgext_amc_channel_desc_record);
- }
+ offset += FRU_PICMGEXT_AMC_CHANNEL_DESC_RECORD_SIZE;
+ }
+
+ for ( ; offset < off + length;)
+ {
+ unsigned int data[2];
+ struct fru_picmgext_amc_link_desc_record * l;
+ l = (struct fru_picmgext_amc_link_desc_record *) &data[0];
+
+ data[0] = fru_data[offset] |
+ (fru_data[offset + 1] << 8) |
+ (fru_data[offset + 2] << 16) |
+ (fru_data[offset + 3] << 24);
+ data[1] = fru_data[offset + 4];

- for ( ; offset < off + length;)
+ printf(" Link Designator: Channel ID: %i\n"
+ " Port Flag 0: %s%s%s%s\n",
+ l->channel_id,
+ (l->port_flag_0)?"o":"-",
+ (l->port_flag_1)?"o":"-",
+ (l->port_flag_2)?"o":"-",
+ (l->port_flag_3)?"o":"-" );
+ switch (l->type)
{
- struct fru_picmgext_amc_link_desc_record * l =
- (struct fru_picmgext_amc_link_desc_record *) &fru_data[offset];
-
- printf(" Link Designator: Channel ID: %i\n"
- " Port Flag 0: %s%s%s%s\n",
- l->channel_id,
- (l->port_flag_0)?"o":"-",
- (l->port_flag_1)?"o":"-",
- (l->port_flag_2)?"o":"-",
- (l->port_flag_3)?"o":"-" );
-
- switch (l->type)
- {
- /* AMC.1 */
- case FRU_PICMGEXT_AMC_LINK_TYPE_PCIE:
- printf(" Link Type: %02x - "
- "AMC.1 PCI Express\n", l->type);
- switch (l->type_ext)
- {
- case AMC_LINK_TYPE_EXT_PCIE_G1_NSSC:
- printf(" Link Type Ext: %i - "
- " Gen 1 capable - non SSC\n", l->type_ext);
- break;
-
- case AMC_LINK_TYPE_EXT_PCIE_G1_SSC:
- printf(" Link Type Ext: %i - "
- " Gen 1 capable - SSC\n", l->type_ext);
- break;
-
- case AMC_LINK_TYPE_EXT_PCIE_G2_NSSC:
- printf(" Link Type Ext: %i - "
- " Gen 2 capable - non SSC\n", l->type_ext);
- break;
- case AMC_LINK_TYPE_EXT_PCIE_G2_SSC:
- printf(" Link Type Ext: %i - "
- " Gen 2 capable - SSC\n", l->type_ext);
- break;
- default:
- printf(" Link Type Ext: %i - "
- " Invalid\n", l->type_ext);
- break;
- }
-
- break;
-
- /* AMC.1 */
- case FRU_PICMGEXT_AMC_LINK_TYPE_PCIE_AS1:
- case FRU_PICMGEXT_AMC_LINK_TYPE_PCIE_AS2:
- printf(" Link Type: %02x - "
- "AMC.1 PCI Express Advanced Switching\n", l->type);
- printf(" Link Type Ext: %i\n", l->type_ext);
- break;
-
- /* AMC.2 */
- case FRU_PICMGEXT_AMC_LINK_TYPE_ETHERNET:
- printf(" Link Type: %02x - "
- "AMC.2 Ethernet\n", l->type);
- switch (l->type_ext)
- {
- case AMC_LINK_TYPE_EXT_ETH_1000_BX:
- printf(" Link Type Ext: %i - "
- " 1000Base-Bx (SerDES Gigabit) Ethernet Link\n", l->type_ext);
- break;
-
- case AMC_LINK_TYPE_EXT_ETH_10G_XAUI:
- printf(" Link Type Ext: %i - "
- " 10Gbit XAUI Ethernet Link\n", l->type_ext);
- break;
-
- default:
- printf(" Link Type Ext: %i - "
- " Invalid\n", l->type_ext);
- break;
- }
- break;
-
- /* AMC.3 */
- case FRU_PICMGEXT_AMC_LINK_TYPE_STORAGE:
- printf(" Link Type: %02x - "
- "AMC.3 Storage\n", l->type);
- switch (l->type_ext)
- {
- case AMC_LINK_TYPE_EXT_STORAGE_FC:
- printf(" Link Type Ext: %i - "
- " Fibre Channel\n", l->type_ext);
- break;
-
- case AMC_LINK_TYPE_EXT_STORAGE_SATA:
- printf(" Link Type Ext: %i - "
- " Serial ATA\n", l->type_ext);
- break;
-
- case AMC_LINK_TYPE_EXT_STORAGE_SAS:
- printf(" Link Type Ext: %i - "
- " Serial Attached SCSI\n", l->type_ext);
- break;
-
- default:
- printf(" Link Type Ext: %i - "
- " Invalid\n", l->type_ext);
- break;
- }
- break;
-
- /* AMC.4 */
- case FRU_PICMGEXT_AMC_LINK_TYPE_RAPIDIO:
- printf(" Link Type: %02x - "
- "AMC.4 Serial Rapid IO\n", l->type);
- printf(" Link Type Ext: %i\n", l->type_ext);
- break;
- default:
- printf(" Link Type: %02x - "
- "reserved or OEM GUID", l->type);
- printf(" Link Type Ext: %i\n", l->type_ext);
- break;
- }
-
- printf(" Link group Id: %i\n", l->group_id);
- printf(" Link Asym Match: %i\n\n",l->asym_match);
-
- offset += sizeof(struct fru_picmgext_amc_link_desc_record);
+ /* AMC.1 */
+ case FRU_PICMGEXT_AMC_LINK_TYPE_PCIE:
+ printf(" Link Type: %02x - "
+ "AMC.1 PCI Express\n", l->type);
+ switch (l->type_ext)
+ {
+ case AMC_LINK_TYPE_EXT_PCIE_G1_NSSC:
+ printf(" Link Type Ext: %i - "
+ " Gen 1 capable - non SSC\n", l->type_ext);
+ break;
+
+ case AMC_LINK_TYPE_EXT_PCIE_G1_SSC:
+ printf(" Link Type Ext: %i - "
+ " Gen 1 capable - SSC\n", l->type_ext);
+ break;
+
+ case AMC_LINK_TYPE_EXT_PCIE_G2_NSSC:
+ printf(" Link Type Ext: %i - "
+ " Gen 2 capable - non SSC\n", l->type_ext);
+ break;
+ case AMC_LINK_TYPE_EXT_PCIE_G2_SSC:
+ printf(" Link Type Ext: %i - "
+ " Gen 2 capable - SSC\n", l->type_ext);
+ break;
+ default:
+ printf(" Link Type Ext: %i - "
+ " Invalid\n", l->type_ext);
+ break;
+ }
+
+ break;
+
+ /* AMC.1 */
+ case FRU_PICMGEXT_AMC_LINK_TYPE_PCIE_AS1:
+ case FRU_PICMGEXT_AMC_LINK_TYPE_PCIE_AS2:
+ printf(" Link Type: %02x - "
+ "AMC.1 PCI Express Advanced Switching\n", l->type);
+ printf(" Link Type Ext: %i\n", l->type_ext);
+ break;
+
+ /* AMC.2 */
+ case FRU_PICMGEXT_AMC_LINK_TYPE_ETHERNET:
+ printf(" Link Type: %02x - "
+ "AMC.2 Ethernet\n", l->type);
+ switch (l->type_ext)
+ {
+ case AMC_LINK_TYPE_EXT_ETH_1000_BX:
+ printf(" Link Type Ext: %i - "
+ " 1000Base-Bx (SerDES Gigabit) Ethernet Link\n", l->type_ext);
+ break;
+
+ case AMC_LINK_TYPE_EXT_ETH_10G_XAUI:
+ printf(" Link Type Ext: %i - "
+ " 10Gbit XAUI Ethernet Link\n", l->type_ext);
+ break;
+
+ default:
+ printf(" Link Type Ext: %i - "
+ " Invalid\n", l->type_ext);
+ break;
+ }
+ break;
+
+ /* AMC.3 */
+ case FRU_PICMGEXT_AMC_LINK_TYPE_STORAGE:
+ printf(" Link Type: %02x - "
+ "AMC.3 Storage\n", l->type);
+ switch (l->type_ext)
+ {
+ case AMC_LINK_TYPE_EXT_STORAGE_FC:
+ printf(" Link Type Ext: %i - "
+ " Fibre Channel\n", l->type_ext);
+ break;
+
+ case AMC_LINK_TYPE_EXT_STORAGE_SATA:
+ printf(" Link Type Ext: %i - "
+ " Serial ATA\n", l->type_ext);
+ break;
+
+ case AMC_LINK_TYPE_EXT_STORAGE_SAS:
+ printf(" Link Type Ext: %i - "
+ " Serial Attached SCSI\n", l->type_ext);
+ break;
+
+ default:
+ printf(" Link Type Ext: %i - "
+ " Invalid\n", l->type_ext);
+ break;
+ }
+ break;
+
+ /* AMC.4 */
+ case FRU_PICMGEXT_AMC_LINK_TYPE_RAPIDIO:
+ printf(" Link Type: %02x - "
+ "AMC.4 Serial Rapid IO\n", l->type);
+ printf(" Link Type Ext: %i\n", l->type_ext);
+ break;
+ default:
+ printf(" Link Type: %02x - "
+ "reserved or OEM GUID", l->type);
+ printf(" Link Type Ext: %i\n", l->type_ext);
+ break;
}
+
+ printf(" Link group Id: %i\n", l->group_id);
+ printf(" Link Asym Match: %i\n\n",l->asym_match);
+
+ offset += FRU_PICMGEXT_AMC_LINK_DESC_RECORD_SIZE;
+ }
}
break;

@@ -2694,8 +2794,8 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
siteCount = fru_data[offset++];

printf(" AMC.0 extension version: R%d.%d\n",
- (extVersion >> 0)& 0x0F,
- (extVersion >> 4)& 0x0F );
+ (extVersion >> 0)& 0x0F,
+ (extVersion >> 4)& 0x0F );
printf(" Carrier Sie Number Cnt: %d\n", siteCount);

for (i = 0 ; i < siteCount; i++ ){
@@ -2776,7 +2876,7 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
direct_cnt = fru_data[offset++];
printf(" Cnt: Indirect 0x%02x / Direct 0x%02x\n",
indirect_cnt,
- direct_cnt );
+ direct_cnt);

/* indirect desc */
for(j=0; j<indirect_cnt; j++){
@@ -2798,23 +2898,23 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
feature = fru_data[offset++];
family = fru_data[offset++];
accuracy = fru_data[offset++];
- freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
- | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
+ freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
+ | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
offset += 4;
- min_freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
- | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
+ min_freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
+ | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
offset += 4;
max_freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
- | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
+ | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
offset += 4;

printf(" - Feature: 0x%02x - PLL: %x / Asym: %s\n",
- feature,
- (feature > 1) & 1,
- (feature&1)?"Source":"Receiver");
+ feature,
+ (feature > 1) & 1,
+ (feature&1)?"Source":"Receiver");
printf(" Family: 0x%02x - AccLVL: 0x%02x\n", family, accuracy);
- printf(" FRQ: %-9d - min: %-9d - max: %-9d\n",
- (int)freq, (int)min_freq, (int)max_freq);
+ printf(" FRQ: %-9d - min: %-9d - max: %-9d\n",
+ freq, min_freq, max_freq);
}
printf("\n");
}
--
1.7.7
Zdenek Styblik
2013-04-22 08:24:11 UTC
Permalink
Post by Dan Gora
Fixed several bugs with printing out the Carrier and AMC connectivity
information with 'ipmitool -v fru'
There were several bugs that were caused due to using bitfields with
char types in ipmi_fru.h. GCC versions before v4.4 had a bug where
they did not pack the bitfields correctly if the bitfields were not
cleanly aligned with the underlying char type.
Fixed and cleaned up all other PICMG FRU records.
Hello,

hopefully, I haven't missed anything important from this patch. I
think it's safe to say one lesson, and "rule", from this patch is not
to mix fixes and indentation/formatting changes together.
Anyway, that's not the reason for this e-mail. My impression of
ipmi_fru_picmg_ext_print() is - it's a hell part of the code. And what
I don't understand is why it isn't split into functions. Three levels
of switch(), really? It definitely can and should be split. And hell,
I'd do it eventually, but only if somebody can test it. I don't posses
hardware to test it. Indentation is really ugly in this part(including
three levels of switch() and alike), resp. way beyond acceptable.
However, the solution isn't to remove indentation and cram the code.
The solution seems to be split this mammoth into smaller
print/whatever functions.
How about it?

Regards,
Z.
Post by Dan Gora
---
ipmitool/include/ipmitool/ipmi_fru.h | 82 ++--
ipmitool/lib/ipmi_fru.c | 866 +++++++++++++++++++---------------
2 files changed, 526 insertions(+), 422 deletions(-)
diff --git a/ipmitool/include/ipmitool/ipmi_fru.h b/ipmitool/include/ipmitool/ipmi_fru.h
index b69e278..6833dd4 100644
--- a/ipmitool/include/ipmitool/ipmi_fru.h
+++ b/ipmitool/include/ipmitool/ipmi_fru.h
@@ -397,20 +397,21 @@ struct fru_picmgext_amc_link_desc {
#define OEM_SWFW_NBLOCK_OFFSET 0x05
#define OEM_SWFW_FIELD_START_OFFSET 0x06
+#define FRU_PICMGEXT_CHN_DESC_RECORD_SIZE 3
#ifdef HAVE_PRAGMA_PACK
#pragma pack(1)
#endif
struct fru_picmgext_chn_desc {
#ifndef WORDS_BIGENDIAN
- unsigned char remote_slot:8;
- unsigned char remote_chn:5;
- unsigned char local_chn:5;
- unsigned char:6;
+ unsigned int remote_slot:8;
+ unsigned int remote_chn:5;
+ unsigned int local_chn:5;
+ unsigned int res:14;
#else
- unsigned char:6;
- unsigned char local_chn:5;
- unsigned char remote_chn:5;
- unsigned char remote_slot:8;
+ unsigned int res:14;
+ unsigned int local_chn:5;
+ unsigned int remote_chn:5;
+ unsigned int remote_slot:8;
#endif
}ATTRIBUTE_PACKING;
#ifdef HAVE_PRAGMA_PACK
@@ -507,28 +508,30 @@ struct fru_picmgext_amc_p2p_record {
#pragma pack(0)
#endif
+#define FRU_PICMGEXT_AMC_CHANNEL_DESC_RECORD_SIZE 3
#ifdef HAVE_PRAGMA_PACK
#pragma pack(1)
#endif
struct fru_picmgext_amc_channel_desc_record {
#ifndef WORDS_BIGENDIAN
- unsigned char lane0port :5;
- unsigned char lane1port :5;
- unsigned char lane2port :5;
- unsigned char lane3port :5;
- unsigned char /*reserved */ :4;
-#else
- unsigned char /*reserved */ :4;
- unsigned char lane3port :5;
- unsigned char lane2port :5;
- unsigned char lane1port :5;
- unsigned char lane0port :5;
+ unsigned int lane0port :5;
+ unsigned int lane1port :5;
+ unsigned int lane2port :5;
+ unsigned int lane3port :5;
+ unsigned int /* reserved */ :12;
+#else
+ unsigned int /* reserved */ :12;
+ unsigned int lane3port :5;
+ unsigned int lane2port :5;
+ unsigned int lane1port :5;
+ unsigned int lane0port :5;
#endif
}ATTRIBUTE_PACKING;
#ifdef HAVE_PRAGMA_PACK
#pragma pack(0)
#endif
+#define FRU_PICMGEXT_AMC_LINK_DESC_RECORD_SIZE 5
#ifdef HAVE_PRAGMA_PACK
#pragma pack(1)
#endif
@@ -552,27 +555,28 @@ struct fru_picmgext_amc_link_desc_record {
#define AMC_LINK_TYPE_EXT_STORAGE_SATA 0x01
#define AMC_LINK_TYPE_EXT_STORAGE_SAS 0x02
#ifndef WORDS_BIGENDIAN
- unsigned short channel_id :8;
- unsigned short port_flag_0 :1;
- unsigned short port_flag_1 :1;
- unsigned short port_flag_2 :1;
- unsigned short port_flag_3 :1;
- unsigned short type :8;
- unsigned short type_ext :4;
- unsigned short group_id :8;
- unsigned char asym_match :2;
- unsigned char /* reserved */ :6;
+ unsigned int channel_id :8;
+ unsigned int port_flag_0 :1;
+ unsigned int port_flag_1 :1;
+ unsigned int port_flag_2 :1;
+ unsigned int port_flag_3 :1;
+ unsigned int type :8;
+ unsigned int type_ext :4;
+ unsigned int group_id :8;
+ unsigned int asym_match :2;
+ unsigned int /* reserved */ :30;
#else
- unsigned char /* reserved */ :6;
- unsigned char asym_match :2;
- unsigned short group_id :8;
- unsigned short type_ext :4;
- unsigned short type :8;
- unsigned short port_flag_3 :1;
- unsigned short port_flag_2 :1;
- unsigned short port_flag_1 :1;
- unsigned short port_flag_0 :1;
- unsigned short channel_id :8;
+ unsigned int group_id :8;
+ unsigned int type_ext :4;
+ unsigned int type :8;
+ unsigned int port_flag_3 :1;
+ unsigned int port_flag_2 :1;
+ unsigned int port_flag_1 :1;
+ unsigned int port_flag_0 :1;
+ unsigned int channel_id :8;
+
+ unsigned int /* reserved */ :30;
+ unsigned int asym_match :2;
#endif
}ATTRIBUTE_PACKING;
#ifdef HAVE_PRAGMA_PACK
diff --git a/ipmitool/lib/ipmi_fru.c b/ipmitool/lib/ipmi_fru.c
index 059c06f..7af3e5b 100644
--- a/ipmitool/lib/ipmi_fru.c
+++ b/ipmitool/lib/ipmi_fru.c
@@ -2133,7 +2133,7 @@ static int ipmi_fru_picmg_ext_edit(uint8_t * fru_data,
static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
{
struct fru_multirec_oem_header *h;
- int guid_count;
+ int guid_count;
int offset = off;
int start_offset = off;
int i;
@@ -2143,74 +2143,103 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
switch (h->record_id)
{
-
{
uint8_t index;
- struct fru_picmgext_slot_desc * slot_d
- = (struct fru_picmgext_slot_desc*) &fru_data[offset];
+ unsigned int data;
+ struct fru_picmgext_slot_desc *slot_d;
+ slot_d =
+ (struct fru_picmgext_slot_desc*) &fru_data[offset];
offset += sizeof(struct fru_picmgext_slot_desc);
printf(" FRU_PICMG_BACKPLANE_P2P\n");
while (offset <= (start_offset+length)) {
printf("\n");
printf(" Channel Type: ");
- switch ( slot_d -> chan_type )
+ switch (slot_d->chan_type)
{
- printf("PICMG 2.9\n");
- break;
- printf("Single Port Fabric IF\n");
- break;
- printf("Double Port Fabric IF\n");
- break;
- printf("Full Channel Fabric IF\n");
- break;
- printf("Base IF\n");
- break;
- printf("Update Channel IF\n");
- break;
- printf("Unknown IF\n");
- break;
+ printf("PICMG 2.9\n");
+ break;
+ printf("Single Port Fabric IF\n");
+ break;
+ printf("Double Port Fabric IF\n");
+ break;
+ printf("Full Channel Fabric IF\n");
+ break;
+ printf("Base IF\n");
+ break;
+ printf("Update Channel IF\n");
+ break;
+ printf("ShMC Cross Connect\n");
+ break;
+ printf("Unknown IF (0x%x)\n",
+ slot_d->chan_type);
+ break;
}
- printf(" Slot Addr. : %02x\n", slot_d -> slot_addr );
- printf(" Channel Count: %i\n", slot_d -> chn_count);
+ printf(" Slot Addr. : %02x\n",
+ slot_d->slot_addr);
+ printf(" Channel Count: %i\n",
+ slot_d->chn_count);
- for (index = 0; index < (slot_d -> chn_count); index++) {
- struct fru_picmgext_chn_desc * d
- = (struct fru_picmgext_chn_desc *) &fru_data[offset];
+ for (index = 0;
+ index < (slot_d -> chn_count);
+ index++) {
+ struct fru_picmgext_chn_desc *d;
+ data = (fru_data[offset+0]) |
+ (fru_data[offset+1] << 8) |
+ (fru_data[offset+2] << 16);
+ d = (struct fru_picmgext_chn_desc *)
+ &data;
if (verbose)
- printf( " "
- "Chn: %02x -> "
- "Chn: %02x in "
- "Slot: %02x\n",
- d->local_chn, d->remote_chn, d->remote_slot);
+ printf( " "
+ "Local Chn: %02x -> "
+ "Remote Chn: %02x in "
+ "Remote Slot: %02x\n",
+ d->local_chn,
+ d->remote_chn,
+ d->remote_slot);
- offset += sizeof(struct fru_picmgext_chn_desc);
- }
+ offset += FRU_PICMGEXT_CHN_DESC_RECORD_SIZE;
+ }
- slot_d = (struct fru_picmgext_slot_desc*) &fru_data[offset];
- offset += sizeof(struct fru_picmgext_slot_desc);
- }
- }
+ slot_d = (struct fru_picmgext_slot_desc*)
+ &fru_data[offset];
+ offset += sizeof(struct fru_picmgext_slot_desc);
+ }
+ }
break;
{
- unsigned char entries = 0;
- unsigned char i;
-
+ unsigned int hwaddr;
+ unsigned int sitetype;
+ unsigned int sitenum;
+ unsigned int entries;
+ unsigned int i;
+ char *picmg_site_type_strings[] = {
+ "AdvancedTCA Board",
+ "Power Entry",
+ "Shelf FRU Information",
+ "Dedicated ShMC",
+ "Fan Tray",
+ "Fan Filter Tray",
+ "Alarm",
+ "AdvancedMC Module",
+ "PMC",
+ "Rear Transition Module"};
+
printf(" FRU_PICMG_ADDRESS_TABLE\n");
-
printf(" Type/Len: 0x%02x\n", fru_data[offset++]);
printf(" Shelf Addr: ");
for (i=0;i<20;i++) {
@@ -2222,61 +2251,117 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
printf(" Addr Table Entries: 0x%02x\n", entries);
for (i=0; i<entries; i++) {
- printf(" HWAddr: 0x%02x - SiteNum: 0x%02x - SiteType: 0x%02x \n",
- fru_data[offset++], fru_data[offset++], fru_data[offset++]);
+ hwaddr = fru_data[offset];
+ sitenum = fru_data[offset + 1];
+ sitetype = fru_data[offset + 2];
+ printf(
+ " HWAddr: 0x%02x (0x%02x) SiteNum: %d SiteType: 0x%02x %s\n",
+ hwaddr, hwaddr * 2,
+ sitenum, sitetype,
+ (sitetype < 0xa) ?
+ "Reserved");
+ offset += 3;
}
}
break;
{
- unsigned char i,j;
- unsigned char feeds = 0;
-
+ unsigned int i,j;
+ unsigned int feeds;
+ unsigned int entries;
+ unsigned int maxext;
+ unsigned int maxint;
+ unsigned int minexp;
+ unsigned int feedcnt;
+ unsigned int hwaddr;
+ unsigned int id;
+
printf(" FRU_PICMG_SHELF_POWER_DIST\n");
feeds = fru_data[offset++];
- printf(" Number of Power Feeds: 0x%02x\n", feeds);
-
+ printf(
+ " Number of Power Feeds: 0x%02x\n",
+ feeds);
for (i=0; i<feeds; i++) {
- unsigned char entries;
-
- printf(" Max Ext Current: 0x%04x\n", fru_data[offset+0] | (fru_data[offset+1]<<8));
+ printf(" Feed %d:\n", i);
+ maxext = fru_data[offset] |
+ (fru_data[offset+1] << 8);
offset += 2;
- printf(" Max Int Current: 0x%04x\n", fru_data[offset+0] | (fru_data[offset+1]<<8));
+ maxint = fru_data[offset] |
+ (fru_data[offset+1] << 8);
offset += 2;
- printf(" Min Exp Voltage: 0x%02x\n", fru_data[offset++]);
+ minexp = fru_data[offset];
+ offset += 1;
+ entries = fru_data[offset];
+ offset += 1;
+
+ printf(
+ " Max External Current: %d.%d Amps (0x%04x)\n",
+ maxext / 10, maxext % 10, maxext);
+ if (maxint < 0xffff)
+ printf(
+ " Max Internal Current: %d.%d Amps (0x%04x)\n",
+ maxint / 10, maxint % 10,
+ maxint);
+ else
+ printf(
+ " Max Internal Current: Not Specified\n");
- entries = fru_data[offset++];
- printf(" Feed to FRU count: 0x%02x\n", entries);
+ if (minexp >= 0x48 && minexp <= 0x90)
+ printf(
+ " Min Expected Voltage: -%02d.%dV\n",
+ minexp / 2, (minexp % 2) * 5);
+ else
+ printf(
+ " Min Expected Voltage: -36V (actual invalid value 0x%x)\n",
+ 36, minexp);
+ printf(
+ " Feed to FRU count: 0x%02x\n",
+ entries);
+
for (j=0; j<entries; j++) {
- printf(" HW: 0x%02x", fru_data[offset++]);
- printf(" FRU ID: 0x%02x\n", fru_data[offset++]);
+ hwaddr = fru_data[offset++];
+ id = fru_data[offset++];
+ printf(
+ " FRU HW Addr: 0x%02x (0x%02x)",
+ hwaddr, hwaddr * 2);
+ printf(" FRU ID: 0x%02x\n",
+ id);
}
}
-
}
break;
{
- unsigned char i;
- unsigned char count = 0;
+ unsigned int i;
+ unsigned int count;
printf(" FRU_PICMG_SHELF_ACTIVATION\n");
- printf(" Allowance for FRU Act Readiness: 0x%02x\n", fru_data[offset++]);
-
+ printf(
+ " Allowance for FRU Act Readiness: 0x%02x\n",
+ fru_data[offset++]);
+
count = fru_data[offset++];
- printf(" FRU activation and Power Desc Cnt: 0x%02x\n", count);
-
+ printf(
+ " FRU activation and Power Desc Cnt: 0x%02x\n",
+ count);
+
for (i=0; i<count; i++) {
- printf(" HW Addr: 0x%02x ", fru_data[offset++]);
- printf(" FRU ID: 0x%02x ", fru_data[offset++]);
- printf(" Max FRU Power: 0x%04x ", fru_data[offset+0] | (fru_data[offset+1]<<8));
+ printf(" HW Addr: 0x%02x ",
+ fru_data[offset++]);
+ printf(" FRU ID: 0x%02x ",
+ fru_data[offset++]);
+ printf(" Max FRU Power: 0x%04x ",
+ fru_data[offset+0] |
+ (fru_data[offset+1]<<8));
offset += 2;
- printf(" Config: 0x%02x \n", fru_data[offset++]);
+ printf(" Config: 0x%02x \n",
+ fru_data[offset++]);
}
}
break;
@@ -2294,71 +2379,73 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
int j;
printf(" GUID [%2d]: 0x", i);
- for (j=0; j < sizeof(struct fru_picmgext_guid); j++) {
+ for (j=0; j < sizeof(struct fru_picmgext_guid);
+ j++) {
printf("%02x", fru_data[offset+j]);
}
printf("\n");
offset += sizeof(struct fru_picmgext_guid);
}
- printf("\n");
-
- for (; offset < off + length; offset += sizeof(struct fru_picmgext_link_desc)) {
+ printf("\n");
+ for (; offset < off + length;
+ offset += sizeof(struct fru_picmgext_link_desc)) {
/* to solve little endian /big endian problem */
- unsigned long data = (fru_data[offset+0])
- | (fru_data[offset+1] << 8)
- | (fru_data[offset+2] << 16)
- | (fru_data[offset+3] << 24);
-
- struct fru_picmgext_link_desc * d = (struct fru_picmgext_link_desc *) &data;
+ struct fru_picmgext_link_desc *d;
+ unsigned int data = (fru_data[offset+0]) |
+ (fru_data[offset+1] << 8) |
+ (fru_data[offset+2] << 16) |
+ (fru_data[offset+3] << 24);
+ d = (struct fru_picmgext_link_desc *) &data;
- printf(" Link Grouping ID: 0x%02x\n", d->grouping);
- printf(" Link Type Extension: 0x%02x - ", d->ext);
- if (d->type == FRU_PICMGEXT_LINK_TYPE_BASE){
+ printf(" Link Grouping ID: 0x%02x\n",
+ d->grouping);
+ printf(" Link Type Extension: 0x%02x - ",
+ d->ext);
+ if (d->type == FRU_PICMGEXT_LINK_TYPE_BASE) {
switch (d->ext)
{
- printf("10/100/1000BASE-T Link (four-pair)\n");
- break;
- printf("ShMC Cross-connect (two-pair)\n");
- break;
- printf("Unknwon\n");
- break;
+ printf("10/100/1000BASE-T Link (four-pair)\n");
+ break;
+ printf("ShMC Cross-connect (two-pair)\n");
+ break;
+ printf("Unknown\n");
+ break;
}
- }else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_ETHERNET){
+ } else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_ETHERNET){
switch (d->ext)
{
- printf("Fixed 1000Base-BX\n");
- break;
- printf("Fixed 10GBASE-BX4 [XAUI]\n");
- break;
- printf("FC-PI\n");
- break;
- printf("Unknwon\n");
- break;
+ printf("Fixed 1000Base-BX\n");
+ break;
+ printf("Fixed 10GBASE-BX4 [XAUI]\n");
+ break;
+ printf("FC-PI\n");
+ break;
+ printf("Unknown\n");
+ break;
}
-
+
}else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_INFINIBAND){
- printf("Unknwon\n");
+ printf("Unknown\n");
}else if (d->type == FRU_PICMGEXT_LINK_TYPE_FABRIC_STAR){
- printf("Unknwon\n");
+ printf("Unknown\n");
}else if (d->type == FRU_PICMGEXT_LINK_TYPE_PCIE){
- printf("Unknwon\n");
- }else
- {
- printf("Unknwon\n");
+ printf("Unknown\n");
+ }else {
+ printf("Unknown\n");
}
-
- printf(" Link Type: 0x%02x - ",d->type);
- if (d->type == 0 || d->type == 0xff)
- {
+
+ printf(" Link Type: 0x%02x - ",
+ d->type);
+ if (d->type == 0 || d->type == 0xff) {
printf("Reserved\n");
}
else if (d->type >= 0x06 && d->type <= 0xef) {
@@ -2370,48 +2457,52 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
else {
switch (d->type)
{
- printf("PICMG 3.0 Base Interface 10/100/1000\n");
- break;
- printf("PICMG 3.1 Ethernet Fabric Interface\n");
- break;
- printf("PICMG 3.2 Infiniband Fabric Interface\n");
- break;
- printf("PICMG 3.3 Star Fabric Interface\n");
- break;
- printf("PICMG 3.4 PCI Express Fabric Interface\n");
- break;
- printf("Invalid\n");
- break;
+ printf("PICMG 3.0 Base Interface 10/100/1000\n");
+ break;
+ printf("PICMG 3.1 Ethernet Fabric Interface\n");
+ break;
+ printf("PICMG 3.2 Infiniband Fabric Interface\n");
+ break;
+ printf("PICMG 3.3 Star Fabric Interface\n");
+ break;
+ printf("PICMG 3.4 PCI Express Fabric Interface\n");
+ break;
+ printf("Invalid\n");
+ break;
}
}
printf(" Link Designator: \n");
- printf(" Port Flag: 0x%02x\n", d->desig_port);
- printf(" Interface: 0x%02x - ", d->desig_if);
- switch (d->desig_if)
- {
- printf("Base Interface\n");
- break;
- printf("Fabric Interface\n");
- break;
- printf("Update Channel\n");
- break;
- printf("Reserved\n");
- break;
- printf("Invalid");
- break;
- }
- printf(" Channel Number: 0x%02x\n", d->desig_channel);
+ printf(" Port Flag: 0x%02x\n",
+ d->desig_port);
+ printf(
+ " Interface: 0x%02x - ",
+ d->desig_if);
+ switch (d->desig_if)
+ {
+ printf("Base Interface\n");
+ break;
+ printf("Fabric Interface\n");
+ break;
+ printf("Update Channel\n");
+ break;
+ printf("Reserved\n");
+ break;
+ printf("Invalid");
+ break;
+ }
+ printf(" Channel Number: 0x%02x\n",
+ d->desig_channel);
printf("\n");
}
@@ -2424,8 +2515,8 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
current = fru_data[offset];
- (float)current / 10.0f,
- (float)current / 10.0f * 12.0f);
+ (float)current / 10.0f,
+ (float)current / 10.0f * 12.0f);
printf("\n");
}
break;
@@ -2438,248 +2529,257 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
max_current = fru_data[offset];
max_current |= fru_data[++offset]<<8;
- (float)max_current / 10.0f,
- (float)max_current / 10.0f * 12.0f);
-
+ (float)max_current / 10.0f,
+ (float)max_current / 10.0f * 12.0f);
printf(" Module Activation Readiness: %i sec.\n", fru_data[++offset]);
printf(" Descriptor Count: %i\n", fru_data[++offset]);
printf("\n");
for(++offset; offset < off + length; offset += sizeof(struct fru_picmgext_activation_record))
{
- struct fru_picmgext_activation_record * a =
- (struct fru_picmgext_activation_record *) &fru_data[offset];
-
+ struct fru_picmgext_activation_record * a;
+ a = (struct fru_picmgext_activation_record *) &fru_data[offset];
printf(" IPMB-Address: 0x%x\n", a->ibmb_addr);
- printf(" Max. Module Current: %.2f A\n", (float)a->max_module_curr / 10.0f);
+ printf(" Max. Module Current: %.2f A\n",
+ (float)a->max_module_curr / 10.0f);
printf("\n");
}
}
break;
- printf(" FRU_CARRIER_P2P\n");
{
- uint16_t index;
-
- for(; offset < off + length; )
- {
- struct fru_picmgext_carrier_p2p_record * h =
- (struct fru_picmgext_carrier_p2p_record *) &fru_data[offset];
-
- printf("\n");
- printf(" Resource ID: %i", h->resource_id & 0x07);
- printf(" Type: ");
- if ((h->resource_id>>7) == 1) {
- printf("AMC\n");
- } else {
- printf("Local\n");
- }
- printf(" Descriptor Count: %i\n", h->p2p_count);
+ printf(" FRU_CARRIER_P2P\n");
+ uint16_t index;
- offset += sizeof(struct fru_picmgext_carrier_p2p_record);
+ for(; offset < off + length; ) {
+ struct fru_picmgext_carrier_p2p_record * h =
+ (struct fru_picmgext_carrier_p2p_record *) &fru_data[offset];
- for (index = 0; index < h->p2p_count; index++)
- {
- /* to solve little endian /big endian problem */
- unsigned char data[3];
- struct fru_picmgext_carrier_p2p_descriptor * desc;
+ printf("\n");
+ printf(" Resource ID: %i", h->resource_id & 0x07);
+ printf(" Type: ");
+ if ((h->resource_id>>7) == 1) {
+ printf("AMC\n");
+ } else {
+ printf("Local\n");
+ }
+ printf(" Descriptor Count: %i\n", h->p2p_count);
- #ifndef WORDS_BIGENDIAN
- data[0] = fru_data[offset+0];
- data[1] = fru_data[offset+1];
- data[2] = fru_data[offset+2];
- #else
- data[0] = fru_data[offset+2];
- data[1] = fru_data[offset+1];
- data[2] = fru_data[offset+0];
- #endif
+ offset += sizeof(struct fru_picmgext_carrier_p2p_record);
- desc = (struct fru_picmgext_carrier_p2p_descriptor*)&data;
+ for (index = 0; index < h->p2p_count; index++)
+ {
+ /* to solve little endian /big endian problem */
+ unsigned char data[3];
+ struct fru_picmgext_carrier_p2p_descriptor * desc;
- printf(" Port: %02d\t-> Remote Port: %02d\t",
- desc->local_port, desc->remote_port);
- if((desc->remote_resource_id >> 7) == 1)
- printf("[ AMC ID: %02d ]\n", desc->remote_resource_id & 0x0F);
- else
- printf("[ local ID: %02d ]\n", desc->remote_resource_id & 0x0F);
+ #ifndef WORDS_BIGENDIAN
+ data[0] = fru_data[offset+0];
+ data[1] = fru_data[offset+1];
+ data[2] = fru_data[offset+2];
+ #else
+ data[0] = fru_data[offset+2];
+ data[1] = fru_data[offset+1];
+ data[2] = fru_data[offset+0];
+ #endif
+
+ desc = (struct fru_picmgext_carrier_p2p_descriptor*)&data;
+
+ printf(" Port: %02d\t-> Remote Port: %02d\t",
+ desc->local_port, desc->remote_port);
+ if((desc->remote_resource_id >> 7) == 1)
+ printf("[ AMC ID: %02d ]\n", desc->remote_resource_id & 0x0F);
+ else
+ printf("[ local ID: %02d ]\n", desc->remote_resource_id & 0x0F);
- offset += sizeof(struct fru_picmgext_carrier_p2p_descriptor);
- }
+ offset += sizeof(struct fru_picmgext_carrier_p2p_descriptor);
}
}
+ }
break;
- printf(" FRU_AMC_P2P\n");
{
- unsigned int index;
- unsigned char channel_count;
- struct fru_picmgext_amc_p2p_record * h;
-
- guid_count = fru_data[offset++];
- printf(" GUID count: %2d\n", guid_count);
-
- for (i = 0 ; i < guid_count; i++ )
- {
- int j;
- printf(" GUID %2d: ", i);
-
- for (j=0; j < sizeof(struct fru_picmgext_guid); j++) {
- printf("%02x", fru_data[offset+j]);
- }
+ unsigned int index;
+ unsigned char channel_count;
+ struct fru_picmgext_amc_p2p_record * h;
+ printf(" FRU_AMC_P2P\n");
- offset += sizeof(struct fru_picmgext_guid);
- printf("\n");
+ guid_count = fru_data[offset];
+ printf(" GUID count: %2d\n", guid_count);
+
+ for (i = 0 ; i < guid_count; i++ )
+ {
+ int j;
+ printf(" GUID %2d: ", i);
+
+ for (j=0; j < sizeof(struct fru_picmgext_guid); j++) {
+ printf("%02x", fru_data[offset+j]);
}
- h = (struct fru_picmgext_amc_p2p_record *) &fru_data[offset];
- printf(" %s", (h->record_type?"AMC Module:":"On-Carrier Device"));
- printf(" Recource ID: %i\n", h->resource_id);
-
- offset += sizeof(struct fru_picmgext_amc_p2p_record);
-
- channel_count = fru_data[offset++];
- printf(" Descriptor Count: %i\n", channel_count);
-
- for (index=0 ;index < channel_count; index++)
- {
- struct fru_picmgext_amc_channel_desc_record * d =
- (struct fru_picmgext_amc_channel_desc_record *) &fru_data[offset];
-
- printf(" Lane 0 Port: %i\n", d->lane0port);
- printf(" Lane 1 Port: %i\n", d->lane1port);
- printf(" Lane 2 Port: %i\n", d->lane2port);
- printf(" Lane 3 Port: %i\n\n", d->lane3port);
-
+ offset += sizeof(struct fru_picmgext_guid);
+ printf("\n");
+ }
+
+ h = (struct fru_picmgext_amc_p2p_record *) &fru_data[++offset];
+ printf(" %s", (h->record_type?"AMC Module:":"On-Carrier Device"));
+ printf(" Resource ID: %i\n", h->resource_id);
+
+ offset += sizeof(struct fru_picmgext_amc_p2p_record);
+
+ channel_count = fru_data[offset++];
+ printf(" Descriptor Count: %i\n", channel_count);
+
+ for (index=0 ;index < channel_count; index++)
+ {
+ unsigned int data;
+ struct fru_picmgext_amc_channel_desc_record *d;
+
+ /* pack the data in little endian format.
+ * Stupid intel... */
+ data = fru_data[offset] |
+ (fru_data[offset + 1] << 8) |
+ (fru_data[offset + 2] << 16);
+ d = (struct fru_picmgext_amc_channel_desc_record *) &data;
+ printf(" Lane 0 Port: %i\n", d->lane0port);
+ printf(" Lane 1 Port: %i\n", d->lane1port);
+ printf(" Lane 2 Port: %i\n", d->lane2port);
+ printf(" Lane 3 Port: %i\n\n", d->lane3port);
- offset += sizeof(struct fru_picmgext_amc_channel_desc_record);
- }
+ offset += FRU_PICMGEXT_AMC_CHANNEL_DESC_RECORD_SIZE;
+ }
+
+ for ( ; offset < off + length;)
+ {
+ unsigned int data[2];
+ struct fru_picmgext_amc_link_desc_record * l;
+ l = (struct fru_picmgext_amc_link_desc_record *) &data[0];
+
+ data[0] = fru_data[offset] |
+ (fru_data[offset + 1] << 8) |
+ (fru_data[offset + 2] << 16) |
+ (fru_data[offset + 3] << 24);
+ data[1] = fru_data[offset + 4];
- for ( ; offset < off + length;)
+ printf(" Link Designator: Channel ID: %i\n"
+ " Port Flag 0: %s%s%s%s\n",
+ l->channel_id,
+ (l->port_flag_0)?"o":"-",
+ (l->port_flag_1)?"o":"-",
+ (l->port_flag_2)?"o":"-",
+ (l->port_flag_3)?"o":"-" );
+ switch (l->type)
{
- struct fru_picmgext_amc_link_desc_record * l =
- (struct fru_picmgext_amc_link_desc_record *) &fru_data[offset];
-
- printf(" Link Designator: Channel ID: %i\n"
- " Port Flag 0: %s%s%s%s\n",
- l->channel_id,
- (l->port_flag_0)?"o":"-",
- (l->port_flag_1)?"o":"-",
- (l->port_flag_2)?"o":"-",
- (l->port_flag_3)?"o":"-" );
-
- switch (l->type)
- {
- /* AMC.1 */
- printf(" Link Type: %02x - "
- "AMC.1 PCI Express\n", l->type);
- switch (l->type_ext)
- {
- printf(" Link Type Ext: %i - "
- " Gen 1 capable - non SSC\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " Gen 1 capable - SSC\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " Gen 2 capable - non SSC\n", l->type_ext);
- break;
- printf(" Link Type Ext: %i - "
- " Gen 2 capable - SSC\n", l->type_ext);
- break;
- printf(" Link Type Ext: %i - "
- " Invalid\n", l->type_ext);
- break;
- }
-
- break;
-
- /* AMC.1 */
- printf(" Link Type: %02x - "
- "AMC.1 PCI Express Advanced Switching\n", l->type);
- printf(" Link Type Ext: %i\n", l->type_ext);
- break;
-
- /* AMC.2 */
- printf(" Link Type: %02x - "
- "AMC.2 Ethernet\n", l->type);
- switch (l->type_ext)
- {
- printf(" Link Type Ext: %i - "
- " 1000Base-Bx (SerDES Gigabit) Ethernet Link\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " 10Gbit XAUI Ethernet Link\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " Invalid\n", l->type_ext);
- break;
- }
- break;
-
- /* AMC.3 */
- printf(" Link Type: %02x - "
- "AMC.3 Storage\n", l->type);
- switch (l->type_ext)
- {
- printf(" Link Type Ext: %i - "
- " Fibre Channel\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " Serial ATA\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " Serial Attached SCSI\n", l->type_ext);
- break;
-
- printf(" Link Type Ext: %i - "
- " Invalid\n", l->type_ext);
- break;
- }
- break;
-
- /* AMC.4 */
- printf(" Link Type: %02x - "
- "AMC.4 Serial Rapid IO\n", l->type);
- printf(" Link Type Ext: %i\n", l->type_ext);
- break;
- printf(" Link Type: %02x - "
- "reserved or OEM GUID", l->type);
- printf(" Link Type Ext: %i\n", l->type_ext);
- break;
- }
-
- printf(" Link group Id: %i\n", l->group_id);
- printf(" Link Asym Match: %i\n\n",l->asym_match);
-
- offset += sizeof(struct fru_picmgext_amc_link_desc_record);
+ /* AMC.1 */
+ printf(" Link Type: %02x - "
+ "AMC.1 PCI Express\n", l->type);
+ switch (l->type_ext)
+ {
+ printf(" Link Type Ext: %i - "
+ " Gen 1 capable - non SSC\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " Gen 1 capable - SSC\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " Gen 2 capable - non SSC\n", l->type_ext);
+ break;
+ printf(" Link Type Ext: %i - "
+ " Gen 2 capable - SSC\n", l->type_ext);
+ break;
+ printf(" Link Type Ext: %i - "
+ " Invalid\n", l->type_ext);
+ break;
+ }
+
+ break;
+
+ /* AMC.1 */
+ printf(" Link Type: %02x - "
+ "AMC.1 PCI Express Advanced Switching\n", l->type);
+ printf(" Link Type Ext: %i\n", l->type_ext);
+ break;
+
+ /* AMC.2 */
+ printf(" Link Type: %02x - "
+ "AMC.2 Ethernet\n", l->type);
+ switch (l->type_ext)
+ {
+ printf(" Link Type Ext: %i - "
+ " 1000Base-Bx (SerDES Gigabit) Ethernet Link\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " 10Gbit XAUI Ethernet Link\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " Invalid\n", l->type_ext);
+ break;
+ }
+ break;
+
+ /* AMC.3 */
+ printf(" Link Type: %02x - "
+ "AMC.3 Storage\n", l->type);
+ switch (l->type_ext)
+ {
+ printf(" Link Type Ext: %i - "
+ " Fibre Channel\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " Serial ATA\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " Serial Attached SCSI\n", l->type_ext);
+ break;
+
+ printf(" Link Type Ext: %i - "
+ " Invalid\n", l->type_ext);
+ break;
+ }
+ break;
+
+ /* AMC.4 */
+ printf(" Link Type: %02x - "
+ "AMC.4 Serial Rapid IO\n", l->type);
+ printf(" Link Type Ext: %i\n", l->type_ext);
+ break;
+ printf(" Link Type: %02x - "
+ "reserved or OEM GUID", l->type);
+ printf(" Link Type Ext: %i\n", l->type_ext);
+ break;
}
+
+ printf(" Link group Id: %i\n", l->group_id);
+ printf(" Link Asym Match: %i\n\n",l->asym_match);
+
+ offset += FRU_PICMGEXT_AMC_LINK_DESC_RECORD_SIZE;
+ }
}
break;
@@ -2694,8 +2794,8 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
siteCount = fru_data[offset++];
printf(" AMC.0 extension version: R%d.%d\n",
- (extVersion >> 0)& 0x0F,
- (extVersion >> 4)& 0x0F );
+ (extVersion >> 0)& 0x0F,
+ (extVersion >> 4)& 0x0F );
printf(" Carrier Sie Number Cnt: %d\n", siteCount);
for (i = 0 ; i < siteCount; i++ ){
@@ -2776,7 +2876,7 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
direct_cnt = fru_data[offset++];
printf(" Cnt: Indirect 0x%02x / Direct 0x%02x\n",
indirect_cnt,
- direct_cnt );
+ direct_cnt);
/* indirect desc */
for(j=0; j<indirect_cnt; j++){
@@ -2798,23 +2898,23 @@ static void ipmi_fru_picmg_ext_print(uint8_t * fru_data, int off, int length)
feature = fru_data[offset++];
family = fru_data[offset++];
accuracy = fru_data[offset++];
- freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
- | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
+ freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
+ | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
offset += 4;
- min_freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
- | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
+ min_freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
+ | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
offset += 4;
max_freq = (fru_data[offset+0] << 0 ) | (fru_data[offset+1] << 8 )
- | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
+ | (fru_data[offset+2] << 16) | (fru_data[offset+3] << 24);
offset += 4;
printf(" - Feature: 0x%02x - PLL: %x / Asym: %s\n",
- feature,
- (feature > 1) & 1,
- (feature&1)?"Source":"Receiver");
+ feature,
+ (feature > 1) & 1,
+ (feature&1)?"Source":"Receiver");
printf(" Family: 0x%02x - AccLVL: 0x%02x\n", family, accuracy);
- printf(" FRQ: %-9d - min: %-9d - max: %-9d\n",
- (int)freq, (int)min_freq, (int)max_freq);
+ printf(" FRQ: %-9d - min: %-9d - max: %-9d\n",
+ freq, min_freq, max_freq);
}
printf("\n");
}
--
1.7.7
------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
Dan Gora
2013-04-22 17:11:16 UTC
Permalink
On Mon, Apr 22, 2013 at 5:24 AM, Zdenek Styblik
Post by Zdenek Styblik
Hello,
hopefully, I haven't missed anything important from this patch. I
think it's safe to say one lesson, and "rule", from this patch is not
to mix fixes and indentation/formatting changes together.
Anyway, that's not the reason for this e-mail. My impression of
ipmi_fru_picmg_ext_print() is - it's a hell part of the code. And what
I don't understand is why it isn't split into functions. Three levels
of switch(), really? It definitely can and should be split. And hell,
I'd do it eventually, but only if somebody can test it. I don't posses
hardware to test it. Indentation is really ugly in this part(including
three levels of switch() and alike), resp. way beyond acceptable.
However, the solution isn't to remove indentation and cram the code.
The solution seems to be split this mammoth into smaller
print/whatever functions.
How about it?
Hi Zdenek,

I saw what you committed to CVS and all that I can say is "wow, this
guy really does not want me to submit a patch ever again".

Modifying my patch to make the indentation 10x worse, while not
actually reducing the amount of difference, which is supposedly what
your complaint above is all about, then committing it without even
giving me a chance to give any feedback is a total amature move. It's
clear that you don't really have any respect for contributors nor any
real idea about what it takes to write professional code, so count me
out... I'm more than capable of maintaining my own fork of ipmitool
without having to deal with ridiculous code formatting arguments.

Please take my name off this patch. I do not want my name associated with the
mess that you committed.

When I see a commit in cvs which says "run lindent on all code, added
coding style document", then maybe I'll think about contributing back
any of my work.

thanks
dan
Jim Mankovich
2013-04-22 17:40:19 UTC
Permalink
I fully agree with Dan in that there is no good reason to change existing
indentation for code. It simply obfuscates real changes and makes application
of patches problematic.
Post by Dan Gora
On Mon, Apr 22, 2013 at 5:24 AM, Zdenek Styblik
Post by Zdenek Styblik
Hello,
hopefully, I haven't missed anything important from this patch. I
think it's safe to say one lesson, and "rule", from this patch is not
to mix fixes and indentation/formatting changes together.
Anyway, that's not the reason for this e-mail. My impression of
ipmi_fru_picmg_ext_print() is - it's a hell part of the code. And what
I don't understand is why it isn't split into functions. Three levels
of switch(), really? It definitely can and should be split. And hell,
I'd do it eventually, but only if somebody can test it. I don't posses
hardware to test it. Indentation is really ugly in this part(including
three levels of switch() and alike), resp. way beyond acceptable.
However, the solution isn't to remove indentation and cram the code.
The solution seems to be split this mammoth into smaller
print/whatever functions.
How about it?
Hi Zdenek,
I saw what you committed to CVS and all that I can say is "wow, this
guy really does not want me to submit a patch ever again".
Modifying my patch to make the indentation 10x worse, while not
actually reducing the amount of difference, which is supposedly what
your complaint above is all about, then committing it without even
giving me a chance to give any feedback is a total amature move. It's
clear that you don't really have any respect for contributors nor any
real idea about what it takes to write professional code, so count me
out... I'm more than capable of maintaining my own fork of ipmitool
without having to deal with ridiculous code formatting arguments.
Please take my name off this patch. I do not want my name associated with the
mess that you committed.
When I see a commit in cvs which says "run lindent on all code, added
coding style document", then maybe I'll think about contributing back
any of my work.
thanks
dan
------------------------------------------------------------------------------
Precog is a next-generation analytics platform capable of advanced
analytics on semi-structured data. The platform includes APIs for building
apps and a phenomenal toolset for data science. Developers can use
our toolset for easy data analysis & visualization. Get a free account!
http://www2.precog.com/precogplatform/slashdotnewsletter
_______________________________________________
Ipmitool-devel mailing list
https://lists.sourceforge.net/lists/listinfo/ipmitool-devel
Dan Gora
2013-04-22 17:53:34 UTC
Permalink
Post by Jim Mankovich
I fully agree with Dan in that there is no good reason to change existing
indentation for code. It simply obfuscates real changes and makes application
of patches problematic.
In fairness, I did change some of the indentation so that lines would
not wrap 80 columns, but I did it just in the parts of the code which
I was changing. Yes, I probably should have did it in two separate
patches, but I didn't really have time to separate them. For the one
of the other patch series, I did separate the changes from the
formatting changes, but for this one I just didn't have time. If it's
_that_ big of a deal, people can apply the patch and use diff -w or
cvs diff -w to ignore the indentation changes.

The problem is to take those changes that I did, make the formatting
much, much, much worse by ensuring that _all_ lines wrap around 80
columns, add tons of totally unnecessary indentation as if just to
spite me, all with the resulting patch yielding almost exactly the
same number of lines of changes, but with the code looking 10x worse,
_then_ committing it without any ack or feedback or opportunity to fix
anything is just flat out rude and unprofessional.

thanks
dan

Dan Gora
2013-04-01 17:52:33 UTC
Permalink
Hi Jim,

(I'm going to cc the ipmitool-devel list so that other people can see
your review and we don't have duplicate your work)
Dan,
Here are a couple of comments/questions on your code changes in this patch
set. I did
not seen anything broken , but I thought I would give you my thoughts on
your
changes.
You split some longer lines and changed indentation without changing
anything
else. I don't see that your changes are really any better than what was
there before.
Why did you bother making these changes?
Basically because I cannot change or understand code that I cannot
read. Code which assumes that I have a 240 column screen, which puts
error cases for every 'if' statement at the bottom, forcing 20 levels
of indentation, variable levels of tab indentation, etc.. I just have
to clean that stuff up before I can even start looking at something.
To me it's kind of programming 101 to have a half way reasonable
coding style so that people can read your code. The linux kernel
style is not my favorite, but at least it's reasonable and consistent.
The code creating the pointer to the struct fru_picmgext_link_desc * from an
an unsigned
int data in ipmi_fru_picmg_ext_print() is making an assumption that the
sizeof(struct fru_picmgext_link_desc) is equal to the sizeof(int).
Well, no, it's making the assumption that sizeof(struct
fru_picmgext_link_desc) is less than or equal to an unsigned int... A
PICMG channel descriptor is always three bytes long.. I don't see that
changing ever really. PICMG would have to change the specification
and break all existing code.
You could verify this is always the case via a compile time #if construct.
I don't really see how you would even do that.
Another option would be to
make the bit fields in fru_picmgext_link_desc part of a union with an
unsigned int, but
that would require quite a bit of code change.
I don't really see how that would help. The problem was with the way
that unaligned bit fields are packed in gcc 4.1-4.3. With gcc in
those versions this struct would be 5 bytes, not 3. That would not
change by using a union.
I only noticed this because you changed data from unsigned long to unsigned int.
yes because the structure is only 3 bytes long. Recall that a long is
8 bytes on 64 bit machines. An unsigned int is always 4 bytes (unless
we're talking about 16 or 8 bit processors here, which we're not...).
Longs (unsigned or not) should really be avoided unless you're really
prepared to deal with them being different sizes or 32 or 64 bit
machines.
A similar assumption is being made for
struct fru_picmgext_amc_link_desc_record.
Right.. These structures are defined in the specification. There is
no reason to believe that they are ever going to change. If they do,
then we'll change the code, but what I wrote doesn't assume any more
about the size of the structure than was assumed before. It just
works around the bug in gcc where it doesn't know how to pack
unaligned bitfields.
The code as written is correct, so it does not really need to be changed.
I just thought I would point this out to you for future
reference.
thanks for taking the time to look at these.. I appreciate it!

thanks
dan
Zdenek Styblik
2013-04-01 18:04:18 UTC
Permalink
On Mon, Apr 1, 2013 at 7:52 PM, Dan Gora <***@gmail.com> wrote:
[...]
Post by Dan Gora
I only noticed this because you changed data from unsigned long to unsigned int.
yes because the structure is only 3 bytes long. Recall that a long is
8 bytes on 64 bit machines. An unsigned int is always 4 bytes (unless
we're talking about 16 or 8 bit processors here, which we're not...).
Longs (unsigned or not) should really be avoided unless you're really
prepared to deal with them being different sizes or 32 or 64 bit
machines.
Why don't you use int types from "<stdint.h>" that I don't understand.
Is there something wrong about it? Because you'd have equally sized
integers no matter what platform you're currently running at. This
was, actually, one of comments/questions/suggestions I had about
posted patches.

Regards,
Z.
Dan Gora
2013-04-01 18:08:22 UTC
Permalink
Post by Zdenek Styblik
Post by Dan Gora
yes because the structure is only 3 bytes long. Recall that a long is
8 bytes on 64 bit machines. An unsigned int is always 4 bytes (unless
we're talking about 16 or 8 bit processors here, which we're not...).
Longs (unsigned or not) should really be avoided unless you're really
prepared to deal with them being different sizes or 32 or 64 bit
machines.
Why don't you use int types from "<stdint.h>" that I don't understand.
Is there something wrong about it? Because you'd have equally sized
integers no matter what platform you're currently running at. This
was, actually, one of comments/questions/suggestions I had about
posted patches.
Just because I'm not used to using them because I've worked on some
platforms which didn't have it and I'm just accustomed to using the
underlying standard C types. I can change that though, no problem.

thanks,
d
Zdenek Styblik
2013-04-01 18:43:25 UTC
Permalink
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
yes because the structure is only 3 bytes long. Recall that a long is
8 bytes on 64 bit machines. An unsigned int is always 4 bytes (unless
we're talking about 16 or 8 bit processors here, which we're not...).
Longs (unsigned or not) should really be avoided unless you're really
prepared to deal with them being different sizes or 32 or 64 bit
machines.
Why don't you use int types from "<stdint.h>" that I don't understand.
Is there something wrong about it? Because you'd have equally sized
integers no matter what platform you're currently running at. This
was, actually, one of comments/questions/suggestions I had about
posted patches.
Just because I'm not used to using them because I've worked on some
platforms which didn't have it and I'm just accustomed to using the
underlying standard C types. I can change that though, no problem.
thanks,
d
No worries. I also didn't mean it as any kind of enforcement.
Dan Gora
2013-04-02 19:04:55 UTC
Permalink
Actually I just remembered another reason why I hate the stdint.h types:

When compiling on Ubuntu 12.10:

../../lib/ipmi_delloem.c: In function 'ipmi_print_power_consmpt_history':
../../lib/ipmi_delloem.c:4006:13: warning: format '%lld' expects
argument of type 'long long int', but argument 2 has type 'uint64_t'
[-Wformat]

So, apparently -Wformat is smart enough to check the type, but not so
smart as to be able to check the underlying type, so you get hundreds
of errors like these if you use the uint64_t rather than the standard
c type 'unsigned long long'.

stdint.h made sense for a period of time when the 16 to 32 bit
processor transition was happening and C was a "new" language and
there was ambiguity about the size of an "int", but really I don't see
the point of it anymore. It just causes tons of issues like this.
It's perfectly possible to write the exact same code which will behave
on all (32 and 64 and even a lot of 16) bit processors without any
ambiguity of the size of a data type as long as you don't use 'long'
(unless you want it to match a pointer size). Especially for a
project like ipmitool which has a pretty limited set of processors
which it has to support.

Anyways, mini-rant against stdint.h over....

thanks
dan
Post by Zdenek Styblik
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
yes because the structure is only 3 bytes long. Recall that a long is
8 bytes on 64 bit machines. An unsigned int is always 4 bytes (unless
we're talking about 16 or 8 bit processors here, which we're not...).
Longs (unsigned or not) should really be avoided unless you're really
prepared to deal with them being different sizes or 32 or 64 bit
machines.
Why don't you use int types from "<stdint.h>" that I don't understand.
Is there something wrong about it? Because you'd have equally sized
integers no matter what platform you're currently running at. This
was, actually, one of comments/questions/suggestions I had about
posted patches.
Just because I'm not used to using them because I've worked on some
platforms which didn't have it and I'm just accustomed to using the
underlying standard C types. I can change that though, no problem.
thanks,
d
No worries. I also didn't mean it as any kind of enforcement.
Zdenek Styblik
2013-04-03 09:40:29 UTC
Permalink
Post by Dan Gora
../../lib/ipmi_delloem.c:4006:13: warning: format '%lld' expects
argument of type 'long long int', but argument 2 has type 'uint64_t'
[-Wformat]
Dan,

I know exactly what you mean. I've hit the same thing, with different
project though, on Debian(and Ubuntu comes from Debian).
Heh. I actually don't know what to write in reply and I had several
versions already.
One thing though. If you don't want to use types from 'stdint.h',
don't. I'm not using all of it and all the time.
Post by Dan Gora
So, apparently -Wformat is smart enough to check the type, but not so
smart as to be able to check the underlying type, so you get hundreds
of errors like these if you use the uint64_t rather than the standard
c type 'unsigned long long'.
Isn't u/int64_t, resp. all 64_t, exactly the case where it makes sense
to use standardized types?

Z.
Post by Dan Gora
stdint.h made sense for a period of time when the 16 to 32 bit
processor transition was happening and C was a "new" language and
there was ambiguity about the size of an "int", but really I don't see
the point of it anymore. It just causes tons of issues like this.
It's perfectly possible to write the exact same code which will behave
on all (32 and 64 and even a lot of 16) bit processors without any
ambiguity of the size of a data type as long as you don't use 'long'
(unless you want it to match a pointer size). Especially for a
project like ipmitool which has a pretty limited set of processors
which it has to support.
Anyways, mini-rant against stdint.h over....
thanks
dan
Post by Zdenek Styblik
Post by Dan Gora
Post by Zdenek Styblik
Post by Dan Gora
yes because the structure is only 3 bytes long. Recall that a long is
8 bytes on 64 bit machines. An unsigned int is always 4 bytes (unless
we're talking about 16 or 8 bit processors here, which we're not...).
Longs (unsigned or not) should really be avoided unless you're really
prepared to deal with them being different sizes or 32 or 64 bit
machines.
Why don't you use int types from "<stdint.h>" that I don't understand.
Is there something wrong about it? Because you'd have equally sized
integers no matter what platform you're currently running at. This
was, actually, one of comments/questions/suggestions I had about
posted patches.
Just because I'm not used to using them because I've worked on some
platforms which didn't have it and I'm just accustomed to using the
underlying standard C types. I can change that though, no problem.
thanks,
d
No worries. I also didn't mean it as any kind of enforcement.
Dan Gora
2013-04-03 17:01:47 UTC
Permalink
Post by Zdenek Styblik
Post by Dan Gora
../../lib/ipmi_delloem.c:4006:13: warning: format '%lld' expects
argument of type 'long long int', but argument 2 has type 'uint64_t'
[-Wformat]
Dan,
I know exactly what you mean. I've hit the same thing, with different
project though, on Debian(and Ubuntu comes from Debian).
Heh. I actually don't know what to write in reply and I had several
versions already.
One thing though. If you don't want to use types from 'stdint.h',
don't. I'm not using all of it and all the time.
I'm willing to change them to be consistent with the other code, but
really I don't use them in new code that I write.
Post by Zdenek Styblik
Post by Dan Gora
So, apparently -Wformat is smart enough to check the type, but not so
smart as to be able to check the underlying type, so you get hundreds
of errors like these if you use the uint64_t rather than the standard
c type 'unsigned long long'.
Isn't u/int64_t, resp. all 64_t, exactly the case where it makes sense
to use standardized types?
well you can use unsigned long long instead of uint64_t. This is 64
bits on both 32 and 64 bit machines. The only types which are
different on 32/64 bit machines are pointers and long/unsigned long
which are 32 bits on 32 bit machines and 64 bits on 64 bit machines.

I think back in the day that 'int' could be 16 bits on some 16 bit
processors, but I don't even think that is true anymore. At least on
the ATMega64/128, an unsigned int is still 32 bits with gcc.

thanks
dan
Continue reading on narkive:
Loading...