Copilot commented on code in PR #13082: URL: https://github.com/apache/trafficserver/pull/13082#discussion_r3062074926
########## plugins/experimental/jax_fingerprint/context_map.h: ########## @@ -0,0 +1,109 @@ +/** @file + * + * Shared context map for jax_fingerprint plugin instances. + * + * When multiple jax_fingerprint.so instances are loaded (e.g., one per + * fingerprinting method), they share a single user arg slot. This map + * stores the JAxContext for each method, keyed by method name. + * + * @section license License + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "config.h" +#include "context.h" + +#include <string> +#include <string_view> +#include <unordered_map> + +/** + * @brief Container holding JAxContext instances for multiple methods. + * + * ATS has a limited number of user arg slots (~4 per type). When loading + * many jax_fingerprint instances, we share a single slot and store all + * contexts in this map, keyed by method name. + */ +class ContextMap +{ +public: + ~ContextMap() + { + for (auto &pair : m_contexts) { + delete pair.second; + } + } + + /** + * @brief Store a context for a method. + * @param[in] method_name The method name (e.g., "JA3", "JA4"). + * @param[in] ctx The context to store. Ownership is transferred. + */ + void + set(std::string_view method_name, JAxContext *ctx) + { + auto it = m_contexts.find(method_name); + if (it != m_contexts.end()) { Review Comment: ContextMap calls m_contexts.find(method_name) where method_name is a std::string_view. That depends on C++20 generic unordered lookup support; elsewhere in this plugin (servernames.contains) there is an __cpp_lib_generic_unordered_lookup fallback. Please add a similar fallback here (or convert to std::string for lookup when the feature macro isn't available) to avoid build failures on standard libraries without generic unordered lookup. ########## plugins/experimental/jax_fingerprint/userarg.h: ########## @@ -30,3 +30,5 @@ int reserve_user_arg(PluginConfig &config); void set_user_arg(void *container, PluginConfig &config, JAxContext *ctx); JAxContext *get_user_arg(void *container, PluginConfig &config); +void cleanup_user_arg(void *container, PluginConfig &config); +void cleanup_user_arg(void *container, PluginConfig &config); Review Comment: Duplicate declaration of cleanup_user_arg() appears twice in this header. Please remove the redundant prototype to avoid confusion and potential -Wredundant-decls warnings. ```suggestion ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
