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.

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

input: add import module for tektronix isf file format #186

Closed
wants to merge 20 commits into from
Closed
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f5ab583
input: add import module for tektronix isf file format
filipkosecek Jun 14, 2022
741a37b
input: fix errors in ISF import module based on pull request reviews
filipkosecek May 30, 2023
166b486
input: make WFID an optional parameter in ISF input module
filipkosecek Jun 8, 2023
e36ebb0
input: add support for unsigned integer data format in ISF module
filipkosecek Jun 8, 2023
4f87b65
input: isf.c: make find_string_value more readable
filipkosecek Jun 11, 2023
2825e37
input: isf.c add input data bounds checking
filipkosecek Jun 11, 2023
0f1e957
input: isf.c add bounds checking for float values
filipkosecek Jun 11, 2023
60b2ac0
input: isf.c Don't require minimum header size
filipkosecek Jun 11, 2023
2407599
input: isf.c don't set confidence when the format does not match
filipkosecek Jun 11, 2023
d8b8f9c
input: isf.c add a brief ISF format description
filipkosecek Jun 11, 2023
332fafd
input/isf.c: perform bounds cheking on bytes per sample value
filipkosecek Jun 13, 2023
482955d
input/isf.c: add error handling when parsing numeric header items
filipkosecek Jun 13, 2023
94e475a
input/isf.c: improve readability
filipkosecek Jun 14, 2023
3704fc1
input/isf.c: wrap the long condition line
filipkosecek Jun 25, 2023
8b9d1e1
input/isf.c: modify the comment
filipkosecek Jun 25, 2023
2081200
input/isf.c: perform bounds checking on curve metadata length
filipkosecek Jun 25, 2023
228a5ee
input/isf.c: reimplement format_match function
filipkosecek Jun 25, 2023
17b954f
input/isf.c: perform bounds checking when reading data samples
filipkosecek Jun 25, 2023
e9f509b
input/isf.c: remove assignment in conditional expressions
filipkosecek Jun 26, 2023
af06f48
input/isf.c: make obtaining curve metadata more obvious
filipkosecek Jun 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 23 additions & 15 deletions src/input/isf.c
Expand Up @@ -177,8 +177,14 @@ static char *find_data_section(GString *buf)
if (offset >= buf->len)
return NULL;

/* Curve metadata length is an ASCII byte, hence -48. */
metadata_length = (size_t) *data_ptr - 48;
/*
* Curve metadata length is encoded as an ASCII
* digit '0' to '9', hence -48.
*/
metadata_length = (size_t) *data_ptr;
if (metadata_length < 48 || metadata_length > 57)
return NULL;
metadata_length -= 48;
data_ptr += 1 + metadata_length;
offset = (size_t) (data_ptr - buf->str);

Expand Down Expand Up @@ -236,7 +242,7 @@ static int find_encoding(const char *buf, size_t buflen)

find_string_value(buf, buflen, value, MAX_ENCODING_STRING_SIZE);

/* "BIN" and "BINARY" are accepted as suggested in a pull request comment. */
/* "BIN" and "BINARY" are accepted. */
if (strcmp(value, "BINARY") != 0 && strcmp(value, "BIN") != 0) {
sr_err("Only binary encoding supported.");
return SR_ERR_NA;
Expand Down Expand Up @@ -468,22 +474,21 @@ static int format_match(GHashTable *metadata, unsigned int *confidence)
char *fn;
size_t fn_len;

/* If the extension is '.isf', it is likely the format matches. */
fn = g_hash_table_lookup(metadata, GINT_TO_POINTER(SR_INPUT_META_FILENAME));
if (fn != NULL && (fn_len = strlen(fn)) >= strlen(default_extension)) {
if (strcmp(fn + fn_len - strlen(default_extension), default_extension) == 0) {
*confidence = 10;
return SR_OK;
}
}

buf = g_hash_table_lookup(metadata, GINT_TO_POINTER(SR_INPUT_META_HEADER));
/* Check if the header contains NR_PT item. */
if (buf == NULL || g_strstr_len(buf->str, buf->len, nr_pt) == NULL)
return SR_ERR;

/* The header contains NR_PT item, the confidence is high. */
*confidence = 50;

/* Increase the confidence if the extension is '.isf'. */
fn = g_hash_table_lookup(metadata, GINT_TO_POINTER(SR_INPUT_META_FILENAME));
if (fn != NULL && (fn_len = strlen(fn)) >= strlen(default_extension)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I didn't catch this last time but you have an assignment inside a conditional, which should be split (I think that is now mentioned in the HACKING doc)

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

if (g_ascii_strcasecmp(fn + fn_len - strlen(default_extension), default_extension) == 0) {
*confidence += 10;
}
}
return SR_OK;
}

Expand Down Expand Up @@ -520,7 +525,8 @@ static float read_int_sample(struct sr_input *in, size_t offset)
inc = in->priv;
bytnr = inc->bytnr;

/* Value bytnr is checked in function "receive". */
if (bytnr > MAX_INT_BYTNR)
return 0;
memcpy(data, in->buf->str + offset, bytnr);
Copy link
Contributor

Choose a reason for hiding this comment

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

bounds check on bytnr before memcpy ? not sure if that's what you mean in the comment on the previous line
[EDIT] ok, yes I see now, it is already checked. But I would probably just replace that comment with the bounds check, even if it's redundant - very inexpensive precaution and 1% easier to review

Copy link
Author

Choose a reason for hiding this comment

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

Bounds checking on bytnr is now performed.

value = 0;
if (inc->byte_order == MSB) {
Expand Down Expand Up @@ -559,7 +565,8 @@ static float read_unsigned_int_sample(struct sr_input *in, size_t offset)

inc = in->priv;

/* Value bytnr is checked in function "receive". */
if (inc->bytnr > MAX_INT_BYTNR)
return 0;
memcpy(data, in->buf->str + offset, inc->bytnr);
if (inc->byte_order == MSB) {
for (i = 0; i < (int) inc->bytnr; ++i) {
Expand Down Expand Up @@ -593,7 +600,8 @@ static float read_float_sample(struct sr_input *in, size_t offset)
bytnr = inc->bytnr;
fp.i = 0;

/* Value bytnr is checked in function "receive". */
if (bytnr > FLOAT_BYTNR)
return 0;
memcpy(data, in->buf->str + offset, bytnr);

if (inc->byte_order == MSB) {
Expand Down