Skip to content

Conversation

@RachelTucker
Copy link
Contributor

Changes

  • Created ds3_uint64_string_map for mapping offsets to checksums in Head Object header parsing.
  • Created ds3_response_header_utils which contains utils for:
    • parsing headers with key ds3-blob-checksum-offset-X into a blob checksum map of blob offset to blob checksum.
    • parsing ds3-blob-checksum-type header for determining blob checksum type.
  • Created new ds3_head_object_response which contains the new blob checksum map and metadata.
    Created free function fords3_head_object_response`.

Tests

  • Added unit tests for new map and header parsing functions.
  • Updated old tests to use new response struct.

return g_hash_table_insert(map->hash, key_cpy, value_cpy);
}

gboolean ds3_uint64_string_map_contains(ds3_uint64_string_map* map, uint64_t* key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method is called publically, it should not return a gboolean, but a ds3_bool instead.

}

// Inserts a safe copy of the key-value pair into the map. Returns true if the key did not exist yet.
gboolean ds3_uint64_string_map_insert(ds3_uint64_string_map* map, const uint64_t* key, const ds3_str* value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method is called publically, it should not return a gboolean, but a ds3_bool instead.

return ds3_str_dup((ds3_str*)value);
}

guint ds3_uint64_string_map_size(ds3_uint64_string_map* map) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method is called publically, it should not return a guint, but a uint64 type instead

#define __DS3_UINT64_T_STRING_MAP__

#include <stdlib.h>
#include <glib.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this file gets included by ds3.h we are leaking the fact that we're using glib. This and other glib reference should be removed from this header.

LIBRARY_API guint ds3_uint64_string_map_size(ds3_uint64_string_map* map);

// Used to iterate through a ds3_uint64_string_map
struct _ds3_uint64_string_map_iter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Several of these can be defined in the .c file which will remove the glib references.


// Converts a ds3_str* containing a checksum value into a ds3_checksum_type*.
// If conversion is not possible, then NULL is returned.
ds3_checksum_type* _convert_str_to_checksum_type(const ds3_log* log, const ds3_str* checksum_str) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this method is only used in this file it should be declared static

}

// Retrieves the offset value at the end of a blob checksum header
uint64_t* _get_offset_from_key(const ds3_str* key) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about maybe needing to be static.

@rpmoore rpmoore merged commit e9d5664 into SpectraLogic:4_1_autogen Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants