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

th/dhcp-options-sort #101

Closed
wants to merge 4 commits into from
Closed

th/dhcp-options-sort #101

wants to merge 4 commits into from

Conversation

thom311
Copy link
Member

@thom311 thom311 commented Apr 21, 2018

At several places, DHCP options are maintained in a hashtable.
Before serializing or printing them, sort them by name.

Otherwise, the order is undefined and unstable. If you call
GetManagedObjects() on D-Bus multiple times, it's a very nice
property if the diff is small and not full not noise.
Otherwise, the output is unstable and changes every time.
Copy link
Contributor

@bengal bengal left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

nm_utils_strdict_to_variant (GHashTable *options)
{
GVariantBuilder builder;
gs_free gs_free const char **keys = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Double 'gs_free'.


options_arr = g_new (char *, nkeys + 1);
for (k = 0; k < nkeys; k++)
options_arr[i++] = g_strdup_printf ("%s = %s", keys[k], (const char *) g_hash_table_lookup (table, keys[k]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need both 'i' and 'k' variables?

@thom311
Copy link
Member Author

thom311 commented Apr 23, 2018

fixed both and merged.

Thanks

@thom311 thom311 closed this Apr 23, 2018
@lkundrak lkundrak deleted the th/dhcp-options-sort branch April 23, 2018 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants