Skip to content

Commit

Permalink
refactor: add gf_strict_atoi function to convert a str into int
Browse files Browse the repository at this point in the history
  • Loading branch information
@touatily
touatily committed Mar 1, 2024
1 parent 28a4db4 commit 48e6376
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 12 deletions.
10 changes: 10 additions & 0 deletions include/gpac/tools.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,16 @@ Compares two timestamps
*/
Bool gf_timestamp_equal(u64 value1, u64 timescale1, u64 value2, u64 timescale2);

/*!
\brief strict convert str into integer
Validate and parse str into integer
\param str text to convert to integer
\param integer to fill
\return GF_TRUE if str represents an integer without any leading space nor extra chars
*/
Bool gf_strict_atoi(const char* str, int* ans);

/*! @} */

/*!
Expand Down
16 changes: 4 additions & 12 deletions src/media_tools/route_dmx.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1160,9 +1160,7 @@ static GF_Err gf_route_service_setup_stsid(GF_ROUTEDmx *routedmx, GF_ROUTEServic
while ((att = gf_list_enum(rs->attributes, &j))) {
if (!stricmp(att->name, "dIpAddr")) dst_ip = att->value;
else if (!stricmp(att->name, "dPort")) {
char* end_ptr;
dst_port = strtol(att->value, &end_ptr, 10);
if(isspace(*att->value) || end_ptr == att->value || *end_ptr != '\0') {
if(! gf_strict_atoi(att->value, &dst_port)) {
GF_LOG(GF_LOG_ERROR, GF_LOG_ROUTE, ("[ROUTE] Service %d wrong dPort value (%s), it should be numeric \n", s->service_id, att->value));
return GF_CORRUPTED_DATA;
} else if(dst_port >= 65536 || dst_port < 0) {
Expand Down Expand Up @@ -1204,9 +1202,7 @@ static GF_Err gf_route_service_setup_stsid(GF_ROUTEDmx *routedmx, GF_ROUTEServic
k=0;
while ((att = gf_list_enum(ls->attributes, &k))) {
if (!strcmp(att->name, "tsi")) {
char* end_ptr;
tsi = strtol(att->value, &end_ptr, 10);
if(isspace(*att->value) || end_ptr == att->value || *end_ptr != '\0') {
if(! gf_strict_atoi(att->value, &tsi)) {
GF_LOG(GF_LOG_ERROR, GF_LOG_ROUTE, ("[ROUTE] Service %d wrong TSI value (%s), it should be numeric \n", s->service_id, att->value));
return GF_CORRUPTED_DATA;
}
Expand Down Expand Up @@ -1262,9 +1258,7 @@ static GF_Err gf_route_service_setup_stsid(GF_ROUTEDmx *routedmx, GF_ROUTEServic
while ((att = gf_list_enum(fdt->attributes, &n))) {
if (!strcmp(att->name, "Content-Location")) rf->filename = gf_strdup(att->value);
else if (!strcmp(att->name, "TOI")) {
char * end_ptr;
rf->toi = strtol(att->value, &end_ptr, 10);
if(isspace(*att->value) || end_ptr == att->value || *end_ptr != '\0') {
if(! gf_strict_atoi(att->value, &rf->toi)) {
GF_LOG(GF_LOG_ERROR, GF_LOG_ROUTE, ("[ROUTE] Service %d wrong TOI value (%s), it should be numeric \n", s->service_id, att->value));
gf_free(rf->filename);
gf_free(rf);
Expand Down Expand Up @@ -1299,9 +1293,7 @@ static GF_Err gf_route_service_setup_stsid(GF_ROUTEDmx *routedmx, GF_ROUTEServic
while ((att = gf_list_enum(fdt->attributes, &n))) {
if (!strcmp(att->name, "Content-Location")) rf->filename = gf_strdup(att->value);
else if (!strcmp(att->name, "TOI")) {
char * end_ptr;
rf->toi = strtol(att->value, &end_ptr, 10);
if(isspace(*att->value) || end_ptr == att->value || *end_ptr != '\0') {
if(! gf_strict_atoi(att->value, &rf->toi)) {
GF_LOG(GF_LOG_ERROR, GF_LOG_ROUTE, ("[ROUTE] Service %d wrong TOI value (%s), it should be numeric \n", s->service_id, att->value));
gf_free(rf->filename);
gf_free(rf);
Expand Down
6 changes: 6 additions & 0 deletions src/utils/error.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2169,3 +2169,9 @@ Bool gf_parse_frac(const char *value, GF_Fraction *frac)
frac->den = (u32) r.den;
return res;
}

Bool gf_strict_atoi(const char* str, int* ans) {
char * end_ptr;

This comment has been minimized.

Sign in to view

Copy link
@rbouqueau

rbouqueau Mar 1, 2024

Member

Don't you you want to initialize this to NULL? It may be assigned only when a number is present, and a comparison then occur on the next line.

This comment has been minimized.

Sign in to view

Copy link
@touatily

touatily Mar 1, 2024

Author Contributor

Thank you for your suggestion! Initializing end_ptr to NULL is indeed a good practice for clarity and defensive programming. Although end_ptr will be updated by strtol() with the address of the first character after the converted number, setting it to NULL at the beginning of the function explicitly establishes its initial state. This makes it clear to readers that end_ptr is intended to be used as a pointer and can be safely dereferenced later in the function. While it may not change the behavior in this specific case, it can improve code readability and maintainability.
*ans = strtol(str, &end_ptr, 10);
return !isspace(*str) && end_ptr != str && *end_ptr == '\0';
}

1 comment on commit 48e6376

@rbouqueau
Copy link
Member

Choose a reason for hiding this comment

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

This function would be a perfect fit for unit tests :) CC @jeanlf @aureliendavid @touatily @soheibthriber

Please sign in to comment.