mike-jumper commented on code in PR #643: URL: https://github.com/apache/guacamole-server/pull/643#discussion_r2916737745
########## src/libguac/eintr.h: ########## @@ -0,0 +1,41 @@ +/* + * 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. + */ + +#ifndef GUAC_EINTR_H +#define GUAC_EINTR_H + +#include <errno.h> + +/** + * Execute the expression and retry if it fails with errno = + * EINTR. Review Comment: Failing doesn't necessarily always mean returning a negative value - we should either document that here or provide a means to define the failure condition. ########## src/libguac/eintr.h: ########## @@ -0,0 +1,41 @@ +/* + * 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. + */ + +#ifndef GUAC_EINTR_H +#define GUAC_EINTR_H + +#include <errno.h> + +/** + * Execute the expression and retry if it fails with errno = + * EINTR. + * + * @param retval + * Return value. + * + * @param expression + * Expression (function) to run. + */ +#define GUAC_EINTR_RETRY(retval, expression) \ Review Comment: This might live better in the established `guacamole/file.h`. ########## src/libguac/eintr.h: ########## @@ -0,0 +1,41 @@ +/* + * 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. + */ + +#ifndef GUAC_EINTR_H +#define GUAC_EINTR_H + +#include <errno.h> + +/** + * Execute the expression and retry if it fails with errno = + * EINTR. + * + * @param retval + * Return value. Review Comment: We should elaborate here such that it's clear how this function should be called. From the perspective of someone needing to provide a value for the `retval` parameter, it's unclear what "return value" means without reading through and understanding the implementation of this macro. ########## src/libguac/socket-wsa.c: ########## @@ -93,7 +93,10 @@ ssize_t guac_socket_wsa_write(guac_socket* socket, /* Write until completely written */ while (count > 0) { - int retval = send(data->sock, buffer, count, 0); + int retval; + do { + retval = send(data->sock, buffer, count, 0); + } while (retval < 0 && WSAGetLastError() == WSAEINTR); Review Comment: If we allow the macro to include the failure test, this could also be turned into an invocation of that macro. We could even continue to provide `GUAC_EINTR_RETRY()` as a convenience macro leveraging the broader macro... something like `GUAC_RETRY_UNTIL()` and `GUAC_RETRY_IGNORE_EINTR()`, maybe. -- 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]
