Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Misc fixes for qrild #21

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Misc fixes for qrild #21

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca18508
qmi: export packet header
calebccff Jun 7, 2022
d86dacc
lib/logging.h: extern C
calebccff Jul 18, 2022
4246d2a
Add support for the qmi_tlv based QMI accessor
calebccff Jul 18, 2022
9e9b6ba
qmi_tlv: fix dump and improve formatting
calebccff Jul 27, 2022
8ade7c5
tlv_dump: fix off-by-1, add string decode, add dump helper
calebccff Aug 3, 2022
b501651
drop tlv_dump stuff, it should be implemented where needed
calebccff Aug 4, 2022
bec5bb2
libqrtr: add qmi_tlv_msg_name to map msg_id to name
calebccff Aug 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions lib/libqrtr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,20 @@ struct qrtr_packet {
size_t data_len;
};

/**
* qmi_header - wireformat header of QMI messages
* @type: type of message
* @txn_id: transaction id
* @msg_id: message id
* @msg_len: length of message payload following header
*/
struct qmi_header {
uint8_t type;
uint16_t txn_id;
uint16_t msg_id;
uint16_t msg_len;
} __attribute__((packed));

#define DEFINE_QRTR_PACKET(pkt, size) \
char pkt ## _buf[size]; \
struct qrtr_packet pkt = { .data = pkt ##_buf, \
Expand Down Expand Up @@ -141,7 +155,10 @@ int qrtr_poll(int sock, unsigned int ms);
int qrtr_decode(struct qrtr_packet *dest, void *buf, size_t len,
const struct sockaddr_qrtr *sq);

const struct qmi_header *qmi_get_header(const struct qrtr_packet *pkt);
int qmi_decode_header(const struct qrtr_packet *pkt, unsigned int *msg_id);
int qmi_decode_header2(const struct qrtr_packet *pkt, unsigned int *msg_id, unsigned char *type,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that either you could expose the struct qmi_header and qmi_get_head() or implement this qmi_decode_header2(), I don't think we need both ways to do the same thing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

unsigned short *txn_id);
int qmi_decode_message(void *c_struct, unsigned int *txn,
const struct qrtr_packet *pkt,
int type, int id, struct qmi_elem_info *ei);
Expand Down
45 changes: 27 additions & 18 deletions lib/qmi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,6 @@
#include "libqrtr.h"
#include "logging.h"

/**
* qmi_header - wireformat header of QMI messages
* @type: type of message
* @txn_id: transaction id
* @msg_id: message id
* @msg_len: length of message payload following header
*/
struct qmi_header {
uint8_t type;
uint16_t txn_id;
uint16_t msg_id;
uint16_t msg_len;
} __attribute__((packed));


#define QMI_ENCDEC_ENCODE_TLV(type, length, p_dst) do { \
*p_dst++ = type; \
*p_dst++ = ((uint8_t)((length) & 0xFF)); \
Expand Down Expand Up @@ -794,20 +779,44 @@ ssize_t qmi_encode_message(struct qrtr_packet *pkt, int type, int msg_id,
return pkt->data_len;
}

int qmi_decode_header(const struct qrtr_packet *pkt, unsigned int *msg_id)
const struct qmi_header *qmi_get_header(const struct qrtr_packet *pkt)
{
const struct qmi_header *qmi = pkt->data;

if (qmi->msg_len != pkt->data_len - sizeof(*qmi)) {
LOGW("[RMTFS] Invalid length of incoming qmi request\n");
return -EINVAL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While being correct, this is unrelated to qmi_header rework. Please split to a separate commit.

LOGW("[QRTR] Invalid length of incoming qmi request\n");
return NULL;
}

return qmi;
}

int qmi_decode_header(const struct qrtr_packet *pkt, unsigned int *msg_id)
{
const struct qmi_header *qmi = qmi_get_header(pkt);

*msg_id = qmi->msg_id;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

qmi_get_header() will return NULL on failure, in which cause this will fault. Better check for !qmi and return -EINVAL in this case.


return 0;
}

int qmi_decode_header2(const struct qrtr_packet *pkt, unsigned int *msg_id, unsigned char *type,
unsigned short *txn_id)
{
const struct qmi_header *qmi = qmi_get_header(pkt);
if (!qmi)
return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently these errors return -EINVAL, it could be argued that returning -1 and setting errno is more idiomatic - but I think you should stick to the current semantics.


if (type)
*type = qmi->type;
if (msg_id)
*msg_id = qmi->msg_id;
if (txn_id)
*txn_id = qmi->txn_id;

return 0;
}

/**
* qmi_decode_message() - Decode QMI encoded message to C structure
* @buf: Buffer with encoded message
Expand Down