[MDEV-15736] mariadb-10.2.14/client/mysqladmin.cc:1586]: (style) Suspicious condition Created: 2018-03-29  Updated: 2023-07-12  Resolved: 2023-07-12

Status: Closed
Project: MariaDB Server
Component/s: Scripts & Clients
Affects Version/s: 5.5, 10.0, 10.1, 10.2, 10.3
Fix Version/s: 11.2.1

Type: Bug Priority: Minor
Reporter: David Binderman Assignee: Andrew Hutchings
Resolution: Fixed Votes: 0
Labels: Papercut


 Description   

[mariadb-10.2.14/client/mysqladmin.cc:1586]: (style) Suspicious condition (assignment + comparison); Clarify expression with parentheses.

Source code is

  if ((length=(uint) strlen(buff) > ex_val_max_len[row]) && ex_status_printed)

Maybe better code

  if ((length=(uint) strlen(buff)) > ex_val_max_len[row] && ex_status_printed)

UPDATE
This is clearly operator precedence bug and should be resolved in this MDEV.
However, additional testing is needed for vertical option mysqladmin -E extended-status for mysqladmin, that is currently not working.
Validate the result with horizontal output mysqladmin --relative --sleep=1 extended-status | grep -v " 0 ", that should work.
Decide should we patch this feature or remove it, since it is not reported as non-working as part of new MDEV.

 Comments   
Comment by Elena Stepanova [ 2018-10-04 ]

The old code is still there, so I'm switching it to Confirmed just to move things forward. I leave it to whoever ends up fixing it to decide whether the new code is actually better.

Comment by Chris Calender (Inactive) [ 2019-02-08 ]

I agree this is better. I added the fix here: https://github.com/MariaDB/server/pull/1168

C Operator Precedence rules listed here: https://en.cppreference.com/w/c/language/operator_precedence

Comment by Sergey Vojtovich [ 2019-03-22 ]

Not really beginner-friendly as the fix impact verification is complicated by completely broken feature.

Comment by Anel Husakovic [ 2023-02-08 ]

After debuggin, noticed that when using mysqladmin -E extended-status, function mysql_fetch_row() returns null and function print_relative_row_vert never gets called.
So, yes we can update this code, to handle operator precedance, but the decision about the --vertical, -E option should be taken into consideration to design properly or to remove.

Comment by Andrew Hutchings [ 2023-02-15 ]

Looking at the source code and the source of historic MariaDB and MySQL versions up to present, this function I don't think ever worked as intended or documented. It is difficult to even figure out what the original intention was supposed to look like. I have asked Monty for feedback as to what direction should be taken.

Comment by Andrew Hutchings [ 2023-03-07 ]

Created a PR that kills this feature

Generated at Thu Feb 08 08:23:38 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.